| Message ID | 20201013120151.9777-1-rejithomas@juniper.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | IPv6: sr: Fix End.X nexthop to use oif. | expand |
Hi Reji,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.9 next-20201013]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Reji-Thomas/IPv6-sr-Fix-End-X-nexthop-to-use-oif/20201013-204935
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 865c50e1d279671728c2936cb7680eb89355eeea
config: x86_64-randconfig-s022-20201013 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-rc1-dirty
# https://github.com/0day-ci/linux/commit/8d40085b9b014197ce7a7e8927730796bf50adb0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Reji-Thomas/IPv6-sr-Fix-End-X-nexthop-to-use-oif/20201013-204935
git checkout 8d40085b9b014197ce7a7e8927730796bf50adb0
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
"sparse warnings: (new ones prefixed by >>)"
>> net/ipv6/seg6_local.c:222:5: sparse: sparse: symbol 'seg6_strict_lookup_nexthop' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Tue, 13 Oct 2020 17:31:51 +0530 Reji Thomas wrote: > Currently End.X action doesn't consider the outgoing interface > while looking up the nexthop.This breaks packet path functionality > specifically while using link local address as the End.X nexthop. > The patch fixes this by enforcing End.X action to have both nh6 and > oif and using oif in lookup.It seems this is a day one issue. > > Fixes: 140f04c33bbc ("implement several seg6local actions") > > Signed-off-by: Reji Thomas <rejithomas@juniper.net> You need to respin to add the missing 'static' kbuild bot pointed out. When you do please also edit the fixes tag to include the full subject, it should look like this: Fixes: 140f04c33bbc ("ipv6: sr: implement several seg6local actions") and remove the empty line between the fixes tag and your signoff. > @@ -239,6 +249,8 @@ static int input_action_end(struct sk_buff *skb, struct seg6_local_lwt *slwt) > static int input_action_end_x(struct sk_buff *skb, struct seg6_local_lwt *slwt) > { > struct ipv6_sr_hdr *srh; > + struct net *net = dev_net(skb->dev); > + struct net_device *odev; Please sort the variable declarations longest to shortest.
Hi Reji,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.9 next-20201013]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Reji-Thomas/IPv6-sr-Fix-End-X-nexthop-to-use-oif/20201013-204935
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 865c50e1d279671728c2936cb7680eb89355eeea
config: riscv-randconfig-r035-20201014 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project e7fe3c6dfede8d5781bd000741c3dea7088307a4)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/8d40085b9b014197ce7a7e8927730796bf50adb0
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Reji-Thomas/IPv6-sr-Fix-End-X-nexthop-to-use-oif/20201013-204935
git checkout 8d40085b9b014197ce7a7e8927730796bf50adb0
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from net/ipv6/seg6_local.c:11:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:148:
include/asm-generic/io.h:556:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return inb(addr);
^~~~~~~~~
arch/riscv/include/asm/io.h:54:76: note: expanded from macro 'inb'
#define inb(c) ({ u8 __v; __io_pbr(); __v = readb_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:87:48: note: expanded from macro 'readb_cpu'
#define readb_cpu(c) ({ u8 __r = __raw_readb(c); __r; })
^
In file included from net/ipv6/seg6_local.c:11:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:148:
include/asm-generic/io.h:564:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return inw(addr);
^~~~~~~~~
arch/riscv/include/asm/io.h:55:76: note: expanded from macro 'inw'
#define inw(c) ({ u16 __v; __io_pbr(); __v = readw_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:88:76: note: expanded from macro 'readw_cpu'
#define readw_cpu(c) ({ u16 __r = le16_to_cpu((__force __le16)__raw_readw(c)); __r; })
^
include/uapi/linux/byteorder/little_endian.h:36:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from net/ipv6/seg6_local.c:11:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:148:
include/asm-generic/io.h:572:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return inl(addr);
^~~~~~~~~
arch/riscv/include/asm/io.h:56:76: note: expanded from macro 'inl'
#define inl(c) ({ u32 __v; __io_pbr(); __v = readl_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:89:76: note: expanded from macro 'readl_cpu'
#define readl_cpu(c) ({ u32 __r = le32_to_cpu((__force __le32)__raw_readl(c)); __r; })
^
include/uapi/linux/byteorder/little_endian.h:34:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from net/ipv6/seg6_local.c:11:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:148:
include/asm-generic/io.h:580:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
outb(value, addr);
^~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:58:68: note: expanded from macro 'outb'
#define outb(v,c) ({ __io_pbw(); writeb_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:91:52: note: expanded from macro 'writeb_cpu'
#define writeb_cpu(v, c) ((void)__raw_writeb((v), (c)))
^
In file included from net/ipv6/seg6_local.c:11:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:148:
include/asm-generic/io.h:588:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
outw(value, addr);
^~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:59:68: note: expanded from macro 'outw'
#define outw(v,c) ({ __io_pbw(); writew_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:92:76: note: expanded from macro 'writew_cpu'
#define writew_cpu(v, c) ((void)__raw_writew((__force u16)cpu_to_le16(v), (c)))
^
In file included from net/ipv6/seg6_local.c:11:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:148:
include/asm-generic/io.h:596:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
outl(value, addr);
^~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:60:68: note: expanded from macro 'outl'
#define outl(v,c) ({ __io_pbw(); writel_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:93:76: note: expanded from macro 'writel_cpu'
#define writel_cpu(v, c) ((void)__raw_writel((__force u32)cpu_to_le32(v), (c)))
^
In file included from net/ipv6/seg6_local.c:11:
In file included from include/linux/skbuff.h:31:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:148:
include/asm-generic/io.h:1017:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
~~~~~~~~~~ ^
>> net/ipv6/seg6_local.c:222:5: warning: no previous prototype for function 'seg6_strict_lookup_nexthop' [-Wmissing-prototypes]
int seg6_strict_lookup_nexthop(struct sk_buff *skb,
^
net/ipv6/seg6_local.c:222:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int seg6_strict_lookup_nexthop(struct sk_buff *skb,
^
static
8 warnings generated.
vim +/seg6_strict_lookup_nexthop +222 net/ipv6/seg6_local.c
221
> 222 int seg6_strict_lookup_nexthop(struct sk_buff *skb,
223 struct in6_addr *nhaddr, int oif, u32 tbl_id)
224 {
225 return seg6_lookup_any_nexthop(skb, nhaddr, oif, tbl_id, false);
226 }
227
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c index eba23279912d..1a669f12d56c 100644 --- a/net/ipv6/seg6_local.c +++ b/net/ipv6/seg6_local.c @@ -153,7 +153,7 @@ static void advance_nextseg(struct ipv6_sr_hdr *srh, struct in6_addr *daddr) static int seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, - u32 tbl_id, bool local_delivery) + int oif, u32 tbl_id, bool local_delivery) { struct net *net = dev_net(skb->dev); struct ipv6hdr *hdr = ipv6_hdr(skb); @@ -164,6 +164,7 @@ seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, int dev_flags = 0; fl6.flowi6_iif = skb->dev->ifindex; + fl6.flowi6_oif = oif; fl6.daddr = nhaddr ? *nhaddr : hdr->daddr; fl6.saddr = hdr->saddr; fl6.flowlabel = ip6_flowinfo(hdr); @@ -173,7 +174,10 @@ seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, if (nhaddr) fl6.flowi6_flags = FLOWI_FLAG_KNOWN_NH; - if (!tbl_id) { + if (oif) + flags |= RT6_LOOKUP_F_IFACE; + + if (!tbl_id && !oif) { dst = ip6_route_input_lookup(net, skb->dev, &fl6, skb, flags); } else { struct fib6_table *table; @@ -182,7 +186,7 @@ seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, if (!table) goto out; - rt = ip6_pol_route(net, table, 0, &fl6, skb, flags); + rt = ip6_pol_route(net, table, oif, &fl6, skb, flags); dst = &rt->dst; } @@ -212,7 +216,13 @@ seg6_lookup_any_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, int seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr, u32 tbl_id) { - return seg6_lookup_any_nexthop(skb, nhaddr, tbl_id, false); + return seg6_lookup_any_nexthop(skb, nhaddr, 0, tbl_id, false); +} + +int seg6_strict_lookup_nexthop(struct sk_buff *skb, + struct in6_addr *nhaddr, int oif, u32 tbl_id) +{ + return seg6_lookup_any_nexthop(skb, nhaddr, oif, tbl_id, false); } /* regular endpoint function */ @@ -239,6 +249,8 @@ static int input_action_end(struct sk_buff *skb, struct seg6_local_lwt *slwt) static int input_action_end_x(struct sk_buff *skb, struct seg6_local_lwt *slwt) { struct ipv6_sr_hdr *srh; + struct net *net = dev_net(skb->dev); + struct net_device *odev; srh = get_and_validate_srh(skb); if (!srh) @@ -246,7 +258,11 @@ static int input_action_end_x(struct sk_buff *skb, struct seg6_local_lwt *slwt) advance_nextseg(srh, &ipv6_hdr(skb)->daddr); - seg6_lookup_nexthop(skb, &slwt->nh6, 0); + odev = dev_get_by_index_rcu(net, slwt->oif); + if (!odev) + goto drop; + + seg6_strict_lookup_nexthop(skb, &slwt->nh6, odev->ifindex, 0); return dst_input(skb); @@ -412,7 +428,7 @@ static int input_action_end_dt6(struct sk_buff *skb, skb_set_transport_header(skb, sizeof(struct ipv6hdr)); - seg6_lookup_any_nexthop(skb, NULL, slwt->table, true); + seg6_lookup_any_nexthop(skb, NULL, 0, slwt->table, true); return dst_input(skb); @@ -566,7 +582,8 @@ static struct seg6_action_desc seg6_action_table[] = { }, { .action = SEG6_LOCAL_ACTION_END_X, - .attrs = (1 << SEG6_LOCAL_NH6), + .attrs = ((1 << SEG6_LOCAL_NH6) | + (1 << SEG6_LOCAL_OIF)), .input = input_action_end_x, }, {
Currently End.X action doesn't consider the outgoing interface while looking up the nexthop.This breaks packet path functionality specifically while using link local address as the End.X nexthop. The patch fixes this by enforcing End.X action to have both nh6 and oif and using oif in lookup.It seems this is a day one issue. Fixes: 140f04c33bbc ("implement several seg6local actions") Signed-off-by: Reji Thomas <rejithomas@juniper.net> --- net/ipv6/seg6_local.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-)