| Message ID | eb4e348ec730900a47caeeb08fe4aff903337675.camel@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] RDMA/addr: Refresh neighbour entries upon "rdma_resolve_addr" | expand |
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
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
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
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; }
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(+)