[PATCH] IPv6: sr: Fix End.X nexthop to use oif.

[PATCH] IPv6: sr: Fix End.X nexthop to use oif.

[Reji Thomas]

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(-)

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,
 	},
 	{
-- 
2.17.1

Re: [PATCH] IPv6: sr: Fix End.X nexthop to use oif.

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31628 bytes --]

[RFC PATCH] IPv6: sr: seg6_strict_lookup_nexthop() can be static

[kernel test robot]

Signed-off-by: kernel test robot <lkp@intel.com>
---
 seg6_local.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index 1a669f12d56c9d..e949d24036c2cd 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -219,8 +219,8 @@ int seg6_lookup_nexthop(struct sk_buff *skb,
 	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)
+static 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);
 }

Re: [PATCH] IPv6: sr: Fix End.X nexthop to use oif.

[Jakub Kicinski]

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.

Re: [PATCH] IPv6: sr: Fix End.X nexthop to use oif.

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 9341 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35486 bytes --]