Message ID | 20230428093914.2121131-6-yanjun.zhu@intel.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Fix the problem that rxe can not work in net namespace | expand |
On 4/28/23 04:39, Zhu Yanjun wrote: > From: Zhu Yanjun <yanjun.zhu@linux.dev> > > Originally a global variable is to keep the sock of udp listening > on port 4791. In fact, sock lookup functions can be used to get > the sock. > > Tested-by: Rain River <rain.1986.08.12@gmail.com> > Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev> > --- > drivers/infiniband/sw/rxe/rxe.c | 1 + > drivers/infiniband/sw/rxe/rxe_net.c | 58 ++++++++++++++++++++------- > drivers/infiniband/sw/rxe/rxe_net.h | 5 --- > drivers/infiniband/sw/rxe/rxe_verbs.h | 1 + > 4 files changed, 45 insertions(+), 20 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c > index ebfabc6d6b76..e81c2164d77f 100644 > --- a/drivers/infiniband/sw/rxe/rxe.c > +++ b/drivers/infiniband/sw/rxe/rxe.c > @@ -74,6 +74,7 @@ static void rxe_init_device_param(struct rxe_dev *rxe) > rxe->ndev->dev_addr); > > rxe->max_ucontext = RXE_MAX_UCONTEXT; > + rxe->l_sk6 = NULL; > } > > /* initialize port attributes */ > diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c > index 4cc7de7b115b..b56e2c32fbf7 100644 > --- a/drivers/infiniband/sw/rxe/rxe_net.c > +++ b/drivers/infiniband/sw/rxe/rxe_net.c > @@ -18,8 +18,6 @@ > #include "rxe_net.h" > #include "rxe_loc.h" > > -static struct rxe_recv_sockets recv_sockets; > - > static struct dst_entry *rxe_find_route4(struct rxe_qp *qp, > struct net_device *ndev, > struct in_addr *saddr, > @@ -51,6 +49,23 @@ static struct dst_entry *rxe_find_route6(struct rxe_qp *qp, > { > struct dst_entry *ndst; > struct flowi6 fl6 = { { 0 } }; > + struct rxe_dev *rdev; > + > + rdev = rxe_get_dev_from_net(ndev); > + if (!rdev->l_sk6) { > + struct sock *sk; > + > + rcu_read_lock(); > + sk = udp6_lib_lookup(&init_net, NULL, 0, &in6addr_any, htons(ROCE_V2_UDP_DPORT), 0); This subroutine (and similar udp4) is currently called for every UD packet. This just adds a lot of code to packet processing for UD packets. All to save a global variable. Not sure it's worth it. Bob > + rcu_read_unlock(); > + if (!sk) { > + pr_info("file: %s +%d, error\n", __FILE__, __LINE__); > + return (struct dst_entry *)sk; > + } > + __sock_put(sk); > + rdev->l_sk6 = sk->sk_socket; > + } > + > > memset(&fl6, 0, sizeof(fl6)); > fl6.flowi6_oif = ndev->ifindex; > @@ -58,8 +73,8 @@ static struct dst_entry *rxe_find_route6(struct rxe_qp *qp, > memcpy(&fl6.daddr, daddr, sizeof(*daddr)); > fl6.flowi6_proto = IPPROTO_UDP; > > - ndst = ipv6_stub->ipv6_dst_lookup_flow(sock_net(recv_sockets.sk6->sk), > - recv_sockets.sk6->sk, &fl6, > + ndst = ipv6_stub->ipv6_dst_lookup_flow(dev_net(ndev), > + rdev->l_sk6->sk, &fl6, > NULL); > if (IS_ERR(ndst)) { > rxe_dbg_qp(qp, "no route to %pI6\n", daddr); > @@ -533,15 +548,33 @@ int rxe_net_add(const char *ibdev_name, struct net_device *ndev) > #define SK_REF_FOR_TUNNEL 2 > void rxe_net_del(struct ib_device *dev) > { > - if (refcount_read(&recv_sockets.sk6->sk->sk_refcnt) > SK_REF_FOR_TUNNEL) > - __sock_put(recv_sockets.sk6->sk); > + struct sock *sk; > + > + rcu_read_lock(); > + sk = udp4_lib_lookup(&init_net, 0, 0, htonl(INADDR_ANY), htons(ROCE_V2_UDP_DPORT), 0); > + rcu_read_unlock(); > + if (!sk) > + return; > + > + __sock_put(sk); > + > + if (refcount_read(&sk->sk_refcnt) > SK_REF_FOR_TUNNEL) > + __sock_put(sk); > else > - rxe_release_udp_tunnel(recv_sockets.sk6); > + rxe_release_udp_tunnel(sk->sk_socket); > + > + rcu_read_lock(); > + sk = udp6_lib_lookup(&init_net, NULL, 0, &in6addr_any, htons(ROCE_V2_UDP_DPORT), 0); > + rcu_read_unlock(); > + if (!sk) > + return; > + > + __sock_put(sk); > > - if (refcount_read(&recv_sockets.sk4->sk->sk_refcnt) > SK_REF_FOR_TUNNEL) > - __sock_put(recv_sockets.sk4->sk); > + if (refcount_read(&sk->sk_refcnt) > SK_REF_FOR_TUNNEL) > + __sock_put(sk); > else > - rxe_release_udp_tunnel(recv_sockets.sk4); > + rxe_release_udp_tunnel(sk->sk_socket); > } > #undef SK_REF_FOR_TUNNEL > > @@ -651,10 +684,8 @@ static int rxe_net_ipv4_init(void) > sock = rxe_setup_udp_tunnel(&init_net, htons(ROCE_V2_UDP_DPORT), false); > if (IS_ERR(sock)) { > pr_err("Failed to create IPv4 UDP tunnel\n"); > - recv_sockets.sk4 = NULL; > return -1; > } > - recv_sockets.sk4 = sock; > > return 0; > } > @@ -674,17 +705,14 @@ static int rxe_net_ipv6_init(void) > > sock = rxe_setup_udp_tunnel(&init_net, htons(ROCE_V2_UDP_DPORT), true); > if (PTR_ERR(sock) == -EAFNOSUPPORT) { > - recv_sockets.sk6 = NULL; > pr_warn("IPv6 is not supported, can not create a UDPv6 socket\n"); > return 0; > } > > if (IS_ERR(sock)) { > - recv_sockets.sk6 = NULL; > pr_err("Failed to create IPv6 UDP tunnel\n"); > return -1; > } > - recv_sockets.sk6 = sock; > #endif > return 0; > } > diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h > index f48f22f3353b..027b20e1bab6 100644 > --- a/drivers/infiniband/sw/rxe/rxe_net.h > +++ b/drivers/infiniband/sw/rxe/rxe_net.h > @@ -11,11 +11,6 @@ > #include <net/if_inet6.h> > #include <linux/module.h> > > -struct rxe_recv_sockets { > - struct socket *sk4; > - struct socket *sk6; > -}; > - > int rxe_net_add(const char *ibdev_name, struct net_device *ndev); > void rxe_net_del(struct ib_device *dev); > > diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h > index c269ae2a3224..ac9bb55820a2 100644 > --- a/drivers/infiniband/sw/rxe/rxe_verbs.h > +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h > @@ -396,6 +396,7 @@ struct rxe_dev { > > struct rxe_port port; > struct crypto_shash *tfm; > + struct socket *l_sk6; > }; > > static inline void rxe_counter_inc(struct rxe_dev *rxe, enum rxe_counters index)
在 2023/6/21 4:28, Bob Pearson 写道: > On 4/28/23 04:39, Zhu Yanjun wrote: >> From: Zhu Yanjun <yanjun.zhu@linux.dev> >> >> Originally a global variable is to keep the sock of udp listening >> on port 4791. In fact, sock lookup functions can be used to get >> the sock. >> >> Tested-by: Rain River <rain.1986.08.12@gmail.com> >> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev> >> --- >> drivers/infiniband/sw/rxe/rxe.c | 1 + >> drivers/infiniband/sw/rxe/rxe_net.c | 58 ++++++++++++++++++++------- >> drivers/infiniband/sw/rxe/rxe_net.h | 5 --- >> drivers/infiniband/sw/rxe/rxe_verbs.h | 1 + >> 4 files changed, 45 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c >> index ebfabc6d6b76..e81c2164d77f 100644 >> --- a/drivers/infiniband/sw/rxe/rxe.c >> +++ b/drivers/infiniband/sw/rxe/rxe.c >> @@ -74,6 +74,7 @@ static void rxe_init_device_param(struct rxe_dev *rxe) >> rxe->ndev->dev_addr); >> >> rxe->max_ucontext = RXE_MAX_UCONTEXT; >> + rxe->l_sk6 = NULL; >> } >> >> /* initialize port attributes */ >> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c >> index 4cc7de7b115b..b56e2c32fbf7 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_net.c >> +++ b/drivers/infiniband/sw/rxe/rxe_net.c >> @@ -18,8 +18,6 @@ >> #include "rxe_net.h" >> #include "rxe_loc.h" >> >> -static struct rxe_recv_sockets recv_sockets; >> - >> static struct dst_entry *rxe_find_route4(struct rxe_qp *qp, >> struct net_device *ndev, >> struct in_addr *saddr, >> @@ -51,6 +49,23 @@ static struct dst_entry *rxe_find_route6(struct rxe_qp *qp, >> { >> struct dst_entry *ndst; >> struct flowi6 fl6 = { { 0 } }; >> + struct rxe_dev *rdev; >> + >> + rdev = rxe_get_dev_from_net(ndev); >> + if (!rdev->l_sk6) { >> + struct sock *sk; >> + >> + rcu_read_lock(); >> + sk = udp6_lib_lookup(&init_net, NULL, 0, &in6addr_any, htons(ROCE_V2_UDP_DPORT), 0); > This subroutine (and similar udp4) is currently called for every UD packet. This just adds a lot of > code to packet processing for UD packets. All to save a global variable. Not sure it's worth it. Thanks, Bob. I also noticed this problem. 2 variables rxe_sk4 and rxe_sk6 are added. The 2 functions udp6_lib_lookup and udp4_lib_lookup are removed. Zhu Yanjun > > Bob >> + rcu_read_unlock(); >> + if (!sk) { >> + pr_info("file: %s +%d, error\n", __FILE__, __LINE__); >> + return (struct dst_entry *)sk; >> + } >> + __sock_put(sk); >> + rdev->l_sk6 = sk->sk_socket; >> + } >> + >> >> memset(&fl6, 0, sizeof(fl6)); >> fl6.flowi6_oif = ndev->ifindex; >> @@ -58,8 +73,8 @@ static struct dst_entry *rxe_find_route6(struct rxe_qp *qp, >> memcpy(&fl6.daddr, daddr, sizeof(*daddr)); >> fl6.flowi6_proto = IPPROTO_UDP; >> >> - ndst = ipv6_stub->ipv6_dst_lookup_flow(sock_net(recv_sockets.sk6->sk), >> - recv_sockets.sk6->sk, &fl6, >> + ndst = ipv6_stub->ipv6_dst_lookup_flow(dev_net(ndev), >> + rdev->l_sk6->sk, &fl6, >> NULL); >> if (IS_ERR(ndst)) { >> rxe_dbg_qp(qp, "no route to %pI6\n", daddr); >> @@ -533,15 +548,33 @@ int rxe_net_add(const char *ibdev_name, struct net_device *ndev) >> #define SK_REF_FOR_TUNNEL 2 >> void rxe_net_del(struct ib_device *dev) >> { >> - if (refcount_read(&recv_sockets.sk6->sk->sk_refcnt) > SK_REF_FOR_TUNNEL) >> - __sock_put(recv_sockets.sk6->sk); >> + struct sock *sk; >> + >> + rcu_read_lock(); >> + sk = udp4_lib_lookup(&init_net, 0, 0, htonl(INADDR_ANY), htons(ROCE_V2_UDP_DPORT), 0); >> + rcu_read_unlock(); >> + if (!sk) >> + return; >> + >> + __sock_put(sk); >> + >> + if (refcount_read(&sk->sk_refcnt) > SK_REF_FOR_TUNNEL) >> + __sock_put(sk); >> else >> - rxe_release_udp_tunnel(recv_sockets.sk6); >> + rxe_release_udp_tunnel(sk->sk_socket); >> + >> + rcu_read_lock(); >> + sk = udp6_lib_lookup(&init_net, NULL, 0, &in6addr_any, htons(ROCE_V2_UDP_DPORT), 0); >> + rcu_read_unlock(); >> + if (!sk) >> + return; >> + >> + __sock_put(sk); >> >> - if (refcount_read(&recv_sockets.sk4->sk->sk_refcnt) > SK_REF_FOR_TUNNEL) >> - __sock_put(recv_sockets.sk4->sk); >> + if (refcount_read(&sk->sk_refcnt) > SK_REF_FOR_TUNNEL) >> + __sock_put(sk); >> else >> - rxe_release_udp_tunnel(recv_sockets.sk4); >> + rxe_release_udp_tunnel(sk->sk_socket); >> } >> #undef SK_REF_FOR_TUNNEL >> >> @@ -651,10 +684,8 @@ static int rxe_net_ipv4_init(void) >> sock = rxe_setup_udp_tunnel(&init_net, htons(ROCE_V2_UDP_DPORT), false); >> if (IS_ERR(sock)) { >> pr_err("Failed to create IPv4 UDP tunnel\n"); >> - recv_sockets.sk4 = NULL; >> return -1; >> } >> - recv_sockets.sk4 = sock; >> >> return 0; >> } >> @@ -674,17 +705,14 @@ static int rxe_net_ipv6_init(void) >> >> sock = rxe_setup_udp_tunnel(&init_net, htons(ROCE_V2_UDP_DPORT), true); >> if (PTR_ERR(sock) == -EAFNOSUPPORT) { >> - recv_sockets.sk6 = NULL; >> pr_warn("IPv6 is not supported, can not create a UDPv6 socket\n"); >> return 0; >> } >> >> if (IS_ERR(sock)) { >> - recv_sockets.sk6 = NULL; >> pr_err("Failed to create IPv6 UDP tunnel\n"); >> return -1; >> } >> - recv_sockets.sk6 = sock; >> #endif >> return 0; >> } >> diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h >> index f48f22f3353b..027b20e1bab6 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_net.h >> +++ b/drivers/infiniband/sw/rxe/rxe_net.h >> @@ -11,11 +11,6 @@ >> #include <net/if_inet6.h> >> #include <linux/module.h> >> >> -struct rxe_recv_sockets { >> - struct socket *sk4; >> - struct socket *sk6; >> -}; >> - >> int rxe_net_add(const char *ibdev_name, struct net_device *ndev); >> void rxe_net_del(struct ib_device *dev); >> >> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h >> index c269ae2a3224..ac9bb55820a2 100644 >> --- a/drivers/infiniband/sw/rxe/rxe_verbs.h >> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h >> @@ -396,6 +396,7 @@ struct rxe_dev { >> >> struct rxe_port port; >> struct crypto_shash *tfm; >> + struct socket *l_sk6; >> }; >> >> static inline void rxe_counter_inc(struct rxe_dev *rxe, enum rxe_counters index)
diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c index ebfabc6d6b76..e81c2164d77f 100644 --- a/drivers/infiniband/sw/rxe/rxe.c +++ b/drivers/infiniband/sw/rxe/rxe.c @@ -74,6 +74,7 @@ static void rxe_init_device_param(struct rxe_dev *rxe) rxe->ndev->dev_addr); rxe->max_ucontext = RXE_MAX_UCONTEXT; + rxe->l_sk6 = NULL; } /* initialize port attributes */ diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c index 4cc7de7b115b..b56e2c32fbf7 100644 --- a/drivers/infiniband/sw/rxe/rxe_net.c +++ b/drivers/infiniband/sw/rxe/rxe_net.c @@ -18,8 +18,6 @@ #include "rxe_net.h" #include "rxe_loc.h" -static struct rxe_recv_sockets recv_sockets; - static struct dst_entry *rxe_find_route4(struct rxe_qp *qp, struct net_device *ndev, struct in_addr *saddr, @@ -51,6 +49,23 @@ static struct dst_entry *rxe_find_route6(struct rxe_qp *qp, { struct dst_entry *ndst; struct flowi6 fl6 = { { 0 } }; + struct rxe_dev *rdev; + + rdev = rxe_get_dev_from_net(ndev); + if (!rdev->l_sk6) { + struct sock *sk; + + rcu_read_lock(); + sk = udp6_lib_lookup(&init_net, NULL, 0, &in6addr_any, htons(ROCE_V2_UDP_DPORT), 0); + rcu_read_unlock(); + if (!sk) { + pr_info("file: %s +%d, error\n", __FILE__, __LINE__); + return (struct dst_entry *)sk; + } + __sock_put(sk); + rdev->l_sk6 = sk->sk_socket; + } + memset(&fl6, 0, sizeof(fl6)); fl6.flowi6_oif = ndev->ifindex; @@ -58,8 +73,8 @@ static struct dst_entry *rxe_find_route6(struct rxe_qp *qp, memcpy(&fl6.daddr, daddr, sizeof(*daddr)); fl6.flowi6_proto = IPPROTO_UDP; - ndst = ipv6_stub->ipv6_dst_lookup_flow(sock_net(recv_sockets.sk6->sk), - recv_sockets.sk6->sk, &fl6, + ndst = ipv6_stub->ipv6_dst_lookup_flow(dev_net(ndev), + rdev->l_sk6->sk, &fl6, NULL); if (IS_ERR(ndst)) { rxe_dbg_qp(qp, "no route to %pI6\n", daddr); @@ -533,15 +548,33 @@ int rxe_net_add(const char *ibdev_name, struct net_device *ndev) #define SK_REF_FOR_TUNNEL 2 void rxe_net_del(struct ib_device *dev) { - if (refcount_read(&recv_sockets.sk6->sk->sk_refcnt) > SK_REF_FOR_TUNNEL) - __sock_put(recv_sockets.sk6->sk); + struct sock *sk; + + rcu_read_lock(); + sk = udp4_lib_lookup(&init_net, 0, 0, htonl(INADDR_ANY), htons(ROCE_V2_UDP_DPORT), 0); + rcu_read_unlock(); + if (!sk) + return; + + __sock_put(sk); + + if (refcount_read(&sk->sk_refcnt) > SK_REF_FOR_TUNNEL) + __sock_put(sk); else - rxe_release_udp_tunnel(recv_sockets.sk6); + rxe_release_udp_tunnel(sk->sk_socket); + + rcu_read_lock(); + sk = udp6_lib_lookup(&init_net, NULL, 0, &in6addr_any, htons(ROCE_V2_UDP_DPORT), 0); + rcu_read_unlock(); + if (!sk) + return; + + __sock_put(sk); - if (refcount_read(&recv_sockets.sk4->sk->sk_refcnt) > SK_REF_FOR_TUNNEL) - __sock_put(recv_sockets.sk4->sk); + if (refcount_read(&sk->sk_refcnt) > SK_REF_FOR_TUNNEL) + __sock_put(sk); else - rxe_release_udp_tunnel(recv_sockets.sk4); + rxe_release_udp_tunnel(sk->sk_socket); } #undef SK_REF_FOR_TUNNEL @@ -651,10 +684,8 @@ static int rxe_net_ipv4_init(void) sock = rxe_setup_udp_tunnel(&init_net, htons(ROCE_V2_UDP_DPORT), false); if (IS_ERR(sock)) { pr_err("Failed to create IPv4 UDP tunnel\n"); - recv_sockets.sk4 = NULL; return -1; } - recv_sockets.sk4 = sock; return 0; } @@ -674,17 +705,14 @@ static int rxe_net_ipv6_init(void) sock = rxe_setup_udp_tunnel(&init_net, htons(ROCE_V2_UDP_DPORT), true); if (PTR_ERR(sock) == -EAFNOSUPPORT) { - recv_sockets.sk6 = NULL; pr_warn("IPv6 is not supported, can not create a UDPv6 socket\n"); return 0; } if (IS_ERR(sock)) { - recv_sockets.sk6 = NULL; pr_err("Failed to create IPv6 UDP tunnel\n"); return -1; } - recv_sockets.sk6 = sock; #endif return 0; } diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h index f48f22f3353b..027b20e1bab6 100644 --- a/drivers/infiniband/sw/rxe/rxe_net.h +++ b/drivers/infiniband/sw/rxe/rxe_net.h @@ -11,11 +11,6 @@ #include <net/if_inet6.h> #include <linux/module.h> -struct rxe_recv_sockets { - struct socket *sk4; - struct socket *sk6; -}; - int rxe_net_add(const char *ibdev_name, struct net_device *ndev); void rxe_net_del(struct ib_device *dev); diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.h b/drivers/infiniband/sw/rxe/rxe_verbs.h index c269ae2a3224..ac9bb55820a2 100644 --- a/drivers/infiniband/sw/rxe/rxe_verbs.h +++ b/drivers/infiniband/sw/rxe/rxe_verbs.h @@ -396,6 +396,7 @@ struct rxe_dev { struct rxe_port port; struct crypto_shash *tfm; + struct socket *l_sk6; }; static inline void rxe_counter_inc(struct rxe_dev *rxe, enum rxe_counters index)