Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751588AbdG0KkY (ORCPT ); Thu, 27 Jul 2017 06:40:24 -0400 Received: from mail.kernel.org ([198.145.29.99]:60892 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751125AbdG0KkW (ORCPT ); Thu, 27 Jul 2017 06:40:22 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B3E4C22B5D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=leon@kernel.org Date: Thu, 27 Jul 2017 13:40:19 +0300 From: Leon Romanovsky To: Marcel Apfelbaum Cc: linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org, monis@mellanox.com, dledford@redhat.com, sean.hefty@intel.com, hal.rosenstock@gmail.com, yuval.shaia@oracle.com Subject: Re: [PATCH] drivers/rxe: improve rxe loopback Message-ID: <20170727104019.GC13672@mtr-leonro.local> References: <20170726145248.21677-1-marcel@redhat.com> <20170727073635.GB13672@mtr-leonro.local> <52aeac10-079f-5c3b-5987-14ead00e2646@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="LyciRD1jyfeSSjG0" Content-Disposition: inline In-Reply-To: <52aeac10-079f-5c3b-5987-14ead00e2646@redhat.com> User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5522 Lines: 163 --LyciRD1jyfeSSjG0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Jul 27, 2017 at 12:49:17PM +0300, Marcel Apfelbaum wrote: > On 27/07/2017 10:36, Leon Romanovsky wrote: > > On Wed, Jul 26, 2017 at 05:52:48PM +0300, Marcel Apfelbaum wrote: > > > Currently a packet is marked for loopback only if the source and > > > destination address match. This is not enough when multiple > > > gids are present in rxe's gid table and the traffic is > > > from one gid to another. > > > > > > Fix it by marking the packet for loopback if the destination > > > address appears in rxe's gid table. > > > > > > Signed-off-by: Marcel Apfelbaum > > > --- > > > drivers/infiniband/sw/rxe/rxe_net.c | 47 +++++++++++++++++++++++++++++++++++-- > > > 1 file changed, 45 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c > > > index c3a140e..b76a9a3 100644 > > > --- a/drivers/infiniband/sw/rxe/rxe_net.c > > > +++ b/drivers/infiniband/sw/rxe/rxe_net.c > > > @@ -351,6 +351,27 @@ static void prepare_ipv6_hdr(struct dst_entry *dst, struct sk_buff *skb, > > > ip6h->payload_len = htons(skb->len - sizeof(*ip6h)); > > > } > > > > > > +static inline bool addr4_same_rxe(struct rxe_dev *rxe, struct in_addr *daddr) > > > +{ > > Hi Leon, > Thanks for the review. > > > > > In addition to Moni's comment, no "inline" functions in *.c files, please. > > > > Sure, I simply followed the function on the same file: > static inline int addr_same(struct rxe_dev *rxe, struct rxe_av *av) > I even borrowed the name... > > > > + struct in_device *in_dev; > > > + bool same_rxe = false; > > > + > > > + rcu_read_lock(); > > > + in_dev = __in_dev_get_rcu(rxe->ndev); > > > + if (!in_dev) > > > + goto out; > > > + > > > + for_ifa(in_dev) > > > + if (!memcmp(&ifa->ifa_address, daddr, sizeof(*daddr))) { > > > + same_rxe = true; > > > + goto out; > > > + } > > > + endfor_ifa(in_dev); > > > > I'm afraid that it will decrease performance drastically. One of the > > possible solutions to overcome it, is to check the address of first packet > > only, but it will work for RC only. > > > > How do you know is "the first" packet? > And yes, for UD the performance would decrease, but only > if the netdev has multiple IPs, right? Yes, and first lookup for QP RC will be "first packet". QP RC are created with "static" address. > > I'll ask on Moni's response mail for alternatives. > > Thanks, > Marcel > > > > +out: > > > + rcu_read_unlock(); > > > + return same_rxe; > > > +} > > > + > > > static int prepare4(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, > > > struct sk_buff *skb, struct rxe_av *av) > > > { > > > @@ -367,7 +388,7 @@ static int prepare4(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, > > > return -EHOSTUNREACH; > > > } > > > > > > - if (!memcmp(saddr, daddr, sizeof(*daddr))) > > > + if (addr4_same_rxe(rxe, daddr)) > > > pkt->mask |= RXE_LOOPBACK_MASK; > > > > > > prepare_udp_hdr(skb, htons(RXE_ROCE_V2_SPORT), > > > @@ -384,6 +405,28 @@ static int prepare4(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, > > > return 0; > > > } > > > > > > +static inline bool addr6_same_rxe(struct rxe_dev *rxe, struct in6_addr *daddr) > > > +{ > > > > Ditto > > > > > + struct inet6_dev *in6_dev; > > > + struct inet6_ifaddr *ifp; > > > + bool same_rxe = false; > > > + > > > + in6_dev = in6_dev_get(rxe->ndev); > > > + if (!in6_dev) > > > + return false; > > > + > > > + read_lock_bh(&in6_dev->lock); > > > + list_for_each_entry(ifp, &in6_dev->addr_list, if_list) > > > + if (!memcmp(&ifp->addr, daddr, sizeof(*daddr))) { > > > + same_rxe = true; > > > + goto out; > > > + } > > > +out: > > > + read_unlock_bh(&in6_dev->lock); > > > + in6_dev_put(in6_dev); > > > + return same_rxe; > > > +} > > > + > > > static int prepare6(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, > > > struct sk_buff *skb, struct rxe_av *av) > > > { > > > @@ -398,7 +441,7 @@ static int prepare6(struct rxe_dev *rxe, struct rxe_pkt_info *pkt, > > > return -EHOSTUNREACH; > > > } > > > > > > - if (!memcmp(saddr, daddr, sizeof(*daddr))) > > > + if (addr6_same_rxe(rxe, daddr)) > > > pkt->mask |= RXE_LOOPBACK_MASK; > > > > > > prepare_udp_hdr(skb, htons(RXE_ROCE_V2_SPORT), > > > -- > > > 2.9.4 > > > > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > > > the body of a message to majordomo@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > --LyciRD1jyfeSSjG0 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEkhr/r4Op1/04yqaB5GN7iDZyWKcFAll5wxMACgkQ5GN7iDZy WKevRg/+LVe7cR0ezGzyoEdbYnEwT2bDvAdGWQfHgTv3qam0ssFHE8DeJl/M4S7y 4gH1nMpvkG0yrs+7TKZzFyBIyAI+0w0aX4iGUD/Bih9zUc8+PE76AP9m0lJA59ix hgDKezuiamWiU1S8VdtZRAaD1N3c/8S7GA4RXnUqmqPCw+uMzlC7UiclJYI77qDC gEHCxc64vM/nWwLDm2Q3pEPfPQmoSknESUAPxPJAzhQtnpCMuktcvdf41WmUkm7m CcOId6Lpvx8H/yAgdRv+qfq18gO7GxL74zr7ss5Z9YYnHQOZBNoi40mvq86TE4Qb 5f6FzcLZasYLIK2qbl87QNln/lWuqfYtFbMZsQGdLmv9xKhFh63CszP7P6BWpqut W8CpzXnfZ5zlinkFqDCwQ6yrFkb1/iUdjvFl0al9eEblWZRSavu/VU6kvsugSw8D 7B0yRuOIJ51fQodBs4LU772Zj/uFz4anAmNZRr9ETesNJbjWGfU5HAESse8IbESi ogh/BcscUSmeORd/A2sDzra0+vVLqkcaN4CiLoB+jtDlJctxVzyv4+rB/2cNfpl2 yvEkBhVdGJl6mjTZfCDBg+XxWEATtotrDUrbG1d2u2x/RZ3Gpo8Ch9A2iiua+EaU BMk56xTHerLaPMX11Ed1waHFvgw7mokTmGrNXXur4ZGOC4rlm9M= =itQ1 -----END PGP SIGNATURE----- --LyciRD1jyfeSSjG0--