[PATCH 1/1] RDMA/addr: Refresh neighbour entries upon "rdma_resolve_addr"

[PATCH 1/1] RDMA/addr: Refresh neighbour entries upon "rdma_resolve_addr"

[Gerd Rausch]

Unlike with IPv[46], where "ip_finish_output2" triggers
a refresh of STALE neighbour entries via "neigh_output",
"rdma_resolve_addr" never triggers an update.

If a wrong STALE entry ever enters the cache, it'll remain
wrong forever (unless refreshed via TCP/IP, or otherwise).

Let the cache inconsistency resolve itself by triggering
an update from "rdma_resolve_addr".

Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
---
 drivers/infiniband/core/addr.c | 40 ++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index f253295795f0..db2a86971e25 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -394,6 +394,8 @@ static int addr4_resolve(struct sockaddr *src_sock,
 	__be32 dst_ip = dst_in->sin_addr.s_addr;
 	struct rtable *rt;
 	struct flowi4 fl4;
+	struct net_device *dev;
+	struct neighbour *neigh;
 	int ret;
 
 	memset(&fl4, 0, sizeof(fl4));
@@ -409,6 +411,24 @@ static int addr4_resolve(struct sockaddr *src_sock,
 
 	addr->hoplimit = ip4_dst_hoplimit(&rt->dst);
 
+	/* trigger ARP-entry refresh if necessary,
+	 * the same way "ip_finish_output2" does
+	 */
+	if (addr->bound_dev_if) {
+		dev = dev_get_by_index(addr->net, addr->bound_dev_if);
+	} else {
+		dev = rt->dst.dev;
+		dev_hold(dev);
+	}
+	if (dev) {
+		rcu_read_lock_bh();
+		neigh = __ipv4_neigh_lookup_noref(dev, dst_ip);
+		if (neigh)
+			neigh_event_send(neigh, NULL);
+		rcu_read_unlock_bh();
+		dev_put(dev);
+	}
+
 	*prt = rt;
 	return 0;
 }
@@ -424,6 +444,8 @@ static int addr6_resolve(struct sockaddr *src_sock,
 				(const struct sockaddr_in6 *)dst_sock;
 	struct flowi6 fl6;
 	struct dst_entry *dst;
+	struct net_device *dev;
+	struct neighbour *neigh;
 
 	memset(&fl6, 0, sizeof fl6);
 	fl6.daddr = dst_in->sin6_addr;
@@ -439,6 +461,24 @@ static int addr6_resolve(struct sockaddr *src_sock,
 
 	addr->hoplimit = ip6_dst_hoplimit(dst);
 
+	/* trigger neighbour-entry refresh if necessary,
+	 * the same way "ip6_finish_output2" does
+	 */
+	if (addr->bound_dev_if) {
+		dev = dev_get_by_index(addr->net, addr->bound_dev_if);
+	} else {
+		dev = dst->dev;
+		dev_hold(dev);
+	}
+	if (dev) {
+		rcu_read_lock_bh();
+		neigh = __ipv6_neigh_lookup_noref(dst->dev, &dst_in->sin6_addr);
+		if (neigh)
+			neigh_event_send(neigh, NULL);
+		rcu_read_unlock_bh();
+		dev_put(dev);
+	}
+
 	*pdst = dst;
 	return 0;
 }

Re: [PATCH 1/1] RDMA/addr: Refresh neighbour entries upon "rdma_resolve_addr"

[kernel test robot]

Hi Gerd,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rdma/for-next]
[also build test WARNING on linus/master v5.19-rc1 next-20220607]
[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/intel-lab-lkp/linux/commits/Gerd-Rausch/RDMA-addr-Refresh-neighbour-entries-upon-rdma_resolve_addr/20220607-033902
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next
config: x86_64-randconfig-s021 (https://download.01.org/0day-ci/archive/20220607/202206071656.1c2rEvO3-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-1) 11.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-18-g56afb504-dirty
        # https://github.com/intel-lab-lkp/linux/commit/b804d2f0ef3768bdf684b2769f8d9fd1306476e7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Gerd-Rausch/RDMA-addr-Refresh-neighbour-entries-upon-rdma_resolve_addr/20220607-033902
        git checkout b804d2f0ef3768bdf684b2769f8d9fd1306476e7
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/infiniband/core/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/infiniband/core/addr.c:425:56: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned int [usertype] key @@     got restricted __be32 [usertype] dst_ip @@
   drivers/infiniband/core/addr.c:425:56: sparse:     expected unsigned int [usertype] key
   drivers/infiniband/core/addr.c:425:56: sparse:     got restricted __be32 [usertype] dst_ip

vim +425 drivers/infiniband/core/addr.c

   383	
   384	static int addr4_resolve(struct sockaddr *src_sock,
   385				 const struct sockaddr *dst_sock,
   386				 struct rdma_dev_addr *addr,
   387				 struct rtable **prt)
   388	{
   389		struct sockaddr_in *src_in = (struct sockaddr_in *)src_sock;
   390		const struct sockaddr_in *dst_in =
   391				(const struct sockaddr_in *)dst_sock;
   392	
   393		__be32 src_ip = src_in->sin_addr.s_addr;
   394		__be32 dst_ip = dst_in->sin_addr.s_addr;
   395		struct rtable *rt;
   396		struct flowi4 fl4;
   397		struct net_device *dev;
   398		struct neighbour *neigh;
   399		int ret;
   400	
   401		memset(&fl4, 0, sizeof(fl4));
   402		fl4.daddr = dst_ip;
   403		fl4.saddr = src_ip;
   404		fl4.flowi4_oif = addr->bound_dev_if;
   405		rt = ip_route_output_key(addr->net, &fl4);
   406		ret = PTR_ERR_OR_ZERO(rt);
   407		if (ret)
   408			return ret;
   409	
   410		src_in->sin_addr.s_addr = fl4.saddr;
   411	
   412		addr->hoplimit = ip4_dst_hoplimit(&rt->dst);
   413	
   414		/* trigger ARP-entry refresh if necessary,
   415		 * the same way "ip_finish_output2" does
   416		 */
   417		if (addr->bound_dev_if) {
   418			dev = dev_get_by_index(addr->net, addr->bound_dev_if);
   419		} else {
   420			dev = rt->dst.dev;
   421			dev_hold(dev);
   422		}
   423		if (dev) {
   424			rcu_read_lock_bh();
 > 425			neigh = __ipv4_neigh_lookup_noref(dev, dst_ip);
   426			if (neigh)
   427				neigh_event_send(neigh, NULL);
   428			rcu_read_unlock_bh();
   429			dev_put(dev);
   430		}
   431	
   432		*prt = rt;
   433		return 0;
   434	}
   435	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 1/1] RDMA/addr: Refresh neighbour entries upon "rdma_resolve_addr"

[Leon Romanovsky]

On Mon, Jun 06, 2022 at 12:38:36PM -0700, Gerd Rausch wrote:
> Unlike with IPv[46], where "ip_finish_output2" triggers
> a refresh of STALE neighbour entries via "neigh_output",
> "rdma_resolve_addr" never triggers an update.
> 
> If a wrong STALE entry ever enters the cache, it'll remain
> wrong forever (unless refreshed via TCP/IP, or otherwise).
> 
> Let the cache inconsistency resolve itself by triggering
> an update from "rdma_resolve_addr".
> 
> Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> ---
>  drivers/infiniband/core/addr.c | 40 ++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)

Gerd,

Do you plan to resubmit the patch?
There is a complain from kbuild robot.

Thanks

Re: [PATCH 1/1] RDMA/addr: Refresh neighbour entries upon "rdma_resolve_addr"

[Gerd Rausch]

On Thu, 2022-06-16 at 10:08 +0300, Leon Romanovsky wrote:
> On Mon, Jun 06, 2022 at 12:38:36PM -0700, Gerd Rausch wrote:
> > Unlike with IPv[46], where "ip_finish_output2" triggers
> > a refresh of STALE neighbour entries via "neigh_output",
> > "rdma_resolve_addr" never triggers an update.
> > 
> > If a wrong STALE entry ever enters the cache, it'll remain
> > wrong forever (unless refreshed via TCP/IP, or otherwise).
> > 
> > Let the cache inconsistency resolve itself by triggering
> > an update from "rdma_resolve_addr".
> > 
> > Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
> > ---
> >  drivers/infiniband/core/addr.c | 40 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> 
> Gerd,
> 
> Do you plan to resubmit the patch?
> There is a complain from kbuild robot.
> 
> 

Hi Leon,

The kbuild robot complained about a __be32 passed as u32.
I'll simply add a "(__force u32)" cast as is done in other places.

I didn't want to make a lot of noise by re-submitting for such a simple
change, but I can certainly re-submit, and will do so.

Thanks,

  Gerd