Message ID | 20220822112355.17635-1-guoqing.jiang@linux.dev (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | RDMA/rxe: No need to check IPV6 in rxe_find_route | expand |
Rather than remove the #if guard here, shouldn't it instead be expanded to cover the entire else if (av->network_type == RXE_NETWORK_TYPE_IPV6) { clause? There is no need to check for RXE_NETWORK_TYPE_IPV6, make assignments to the stack-allocated pointers or call rxe_find_route6() unless CONFIG_IPV6 is true. In fact, if CONFIG_IPV6 is false, as rxe_find_route6 would also return NULL, the else clause to the RXE_NETWORK_TYPE_IPV4 check could instead become a simple return NULL; > On Aug 22, 2022, at 5:23 AM, Guoqing Jiang <guoqing.jiang@linux.dev> wrote: > > This check is unnecessary since rxe_find_route6 returns NULL if > CONFIG_IPV6 is disabled. > > Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev> > --- > drivers/infiniband/sw/rxe/rxe_net.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c > index c53f4529f098..b0f31f849144 100644 > --- a/drivers/infiniband/sw/rxe/rxe_net.c > +++ b/drivers/infiniband/sw/rxe/rxe_net.c > @@ -113,11 +113,9 @@ static struct dst_entry *rxe_find_route(struct net_device *ndev, > saddr6 = &av->sgid_addr._sockaddr_in6.sin6_addr; > daddr6 = &av->dgid_addr._sockaddr_in6.sin6_addr; > dst = rxe_find_route6(ndev, saddr6, daddr6); > -#if IS_ENABLED(CONFIG_IPV6) > if (dst) > qp->dst_cookie = > rt6_get_cookie((struct rt6_info *)dst); > -#endif > } > > if (dst && (qp_type(qp) == IB_QPT_RC)) { > -- > 2.31.1 >
Hi Guoqing,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rdma/for-next]
[also build test ERROR on linus/master v6.0-rc2 next-20220822]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Guoqing-Jiang/RDMA-rxe-No-need-to-check-IPV6-in-rxe_find_route/20220822-192520
base: https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next
config: ia64-buildonly-randconfig-r005-20220822 (https://download.01.org/0day-ci/archive/20220823/202208230225.DS04ZlXH-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
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
# https://github.com/intel-lab-lkp/linux/commit/9d154e806104f75aae6c66dfd78ecd5e67c7e00d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Guoqing-Jiang/RDMA-rxe-No-need-to-check-IPV6-in-rxe_find_route/20220822-192520
git checkout 9d154e806104f75aae6c66dfd78ecd5e67c7e00d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/infiniband/sw/rxe/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/infiniband/sw/rxe/rxe_net.c: In function 'rxe_find_route':
>> drivers/infiniband/sw/rxe/rxe_net.c:118:41: error: implicit declaration of function 'rt6_get_cookie' [-Werror=implicit-function-declaration]
118 | rt6_get_cookie((struct rt6_info *)dst);
| ^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/rt6_get_cookie +118 drivers/infiniband/sw/rxe/rxe_net.c
8700e3e7c4857d Moni Shoua 2016-06-16 88
3db2bceb29fd9a Parav Pandit 2018-08-28 89 static struct dst_entry *rxe_find_route(struct net_device *ndev,
4ed6ad1eb30e20 yonatanc 2017-04-20 90 struct rxe_qp *qp,
4ed6ad1eb30e20 yonatanc 2017-04-20 91 struct rxe_av *av)
4ed6ad1eb30e20 yonatanc 2017-04-20 92 {
4ed6ad1eb30e20 yonatanc 2017-04-20 93 struct dst_entry *dst = NULL;
4ed6ad1eb30e20 yonatanc 2017-04-20 94
4ed6ad1eb30e20 yonatanc 2017-04-20 95 if (qp_type(qp) == IB_QPT_RC)
4ed6ad1eb30e20 yonatanc 2017-04-20 96 dst = sk_dst_get(qp->sk->sk);
4ed6ad1eb30e20 yonatanc 2017-04-20 97
b9109b7ddb13a5 Andrew Boyer 2017-08-28 98 if (!dst || !dst_check(dst, qp->dst_cookie)) {
4ed6ad1eb30e20 yonatanc 2017-04-20 99 if (dst)
4ed6ad1eb30e20 yonatanc 2017-04-20 100 dst_release(dst);
4ed6ad1eb30e20 yonatanc 2017-04-20 101
e0d696d201dd5d Jason Gunthorpe 2020-10-15 102 if (av->network_type == RXE_NETWORK_TYPE_IPV4) {
4ed6ad1eb30e20 yonatanc 2017-04-20 103 struct in_addr *saddr;
4ed6ad1eb30e20 yonatanc 2017-04-20 104 struct in_addr *daddr;
4ed6ad1eb30e20 yonatanc 2017-04-20 105
4ed6ad1eb30e20 yonatanc 2017-04-20 106 saddr = &av->sgid_addr._sockaddr_in.sin_addr;
4ed6ad1eb30e20 yonatanc 2017-04-20 107 daddr = &av->dgid_addr._sockaddr_in.sin_addr;
43c9fc509fa59d Martin Wilck 2018-02-14 108 dst = rxe_find_route4(ndev, saddr, daddr);
e0d696d201dd5d Jason Gunthorpe 2020-10-15 109 } else if (av->network_type == RXE_NETWORK_TYPE_IPV6) {
4ed6ad1eb30e20 yonatanc 2017-04-20 110 struct in6_addr *saddr6;
4ed6ad1eb30e20 yonatanc 2017-04-20 111 struct in6_addr *daddr6;
4ed6ad1eb30e20 yonatanc 2017-04-20 112
4ed6ad1eb30e20 yonatanc 2017-04-20 113 saddr6 = &av->sgid_addr._sockaddr_in6.sin6_addr;
4ed6ad1eb30e20 yonatanc 2017-04-20 114 daddr6 = &av->dgid_addr._sockaddr_in6.sin6_addr;
43c9fc509fa59d Martin Wilck 2018-02-14 115 dst = rxe_find_route6(ndev, saddr6, daddr6);
b9109b7ddb13a5 Andrew Boyer 2017-08-28 116 if (dst)
b9109b7ddb13a5 Andrew Boyer 2017-08-28 117 qp->dst_cookie =
b9109b7ddb13a5 Andrew Boyer 2017-08-28 @118 rt6_get_cookie((struct rt6_info *)dst);
4ed6ad1eb30e20 yonatanc 2017-04-20 119 }
24c937b39dfb10 Vijay Immanuel 2018-06-18 120
24c937b39dfb10 Vijay Immanuel 2018-06-18 121 if (dst && (qp_type(qp) == IB_QPT_RC)) {
24c937b39dfb10 Vijay Immanuel 2018-06-18 122 dst_hold(dst);
24c937b39dfb10 Vijay Immanuel 2018-06-18 123 sk_dst_set(qp->sk->sk, dst);
24c937b39dfb10 Vijay Immanuel 2018-06-18 124 }
4ed6ad1eb30e20 yonatanc 2017-04-20 125 }
4ed6ad1eb30e20 yonatanc 2017-04-20 126 return dst;
4ed6ad1eb30e20 yonatanc 2017-04-20 127 }
4ed6ad1eb30e20 yonatanc 2017-04-20 128
On 8/23/22 12:33 AM, William Kucharski wrote: > Rather than remove the #if guard here, shouldn't it instead be expanded > to cover the entire > > else if (av->network_type == RXE_NETWORK_TYPE_IPV6) { > > clause? > > There is no need to check for RXE_NETWORK_TYPE_IPV6, make assignments > to the stack-allocated pointers or call rxe_find_route6() unless > CONFIG_IPV6 is true. > > In fact, if CONFIG_IPV6 is false, as rxe_find_route6 would also return > NULL, the else clause to the RXE_NETWORK_TYPE_IPV4 check could instead > become a simple > > return NULL; Thanks for the suggestion! And it could also resolve the issue reported by LKP too. Thanks, Guoqing
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c index c53f4529f098..b0f31f849144 100644 --- a/drivers/infiniband/sw/rxe/rxe_net.c +++ b/drivers/infiniband/sw/rxe/rxe_net.c @@ -113,11 +113,9 @@ static struct dst_entry *rxe_find_route(struct net_device *ndev, saddr6 = &av->sgid_addr._sockaddr_in6.sin6_addr; daddr6 = &av->dgid_addr._sockaddr_in6.sin6_addr; dst = rxe_find_route6(ndev, saddr6, daddr6); -#if IS_ENABLED(CONFIG_IPV6) if (dst) qp->dst_cookie = rt6_get_cookie((struct rt6_info *)dst); -#endif } if (dst && (qp_type(qp) == IB_QPT_RC)) {
This check is unnecessary since rxe_find_route6 returns NULL if CONFIG_IPV6 is disabled. Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev> --- drivers/infiniband/sw/rxe/rxe_net.c | 2 -- 1 file changed, 2 deletions(-)