On Thu, Jan 24, 2019 at 05:10:18PM +0200, Yuval Shaia wrote:
> On Thu, Jan 24, 2019 at 04:44:34PM +0200, Kamal Heib wrote:
> > Currently a packet is marked for loopback only if the source and
> > destination addresses match, This is not enough when multiple gids are
>
> s/match,/equals.
>
> > present in rxe device's gid table and the traffic is from one gid to
> > another, this patch fixes this by marking the packet for loopback if the
> > destination address found in the rxe's netdevice address list.
>
> s/another, this patch fixes this/ another. Fix it
>
> >
> > Fixes: 8700e3e7c485 ("Soft RoCE driver")
> > Signed-off-by: Marcel Apfelbaum <marcel.apfelbaum@xxxxxxxxx>
> > Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx>
> > ---
> > drivers/infiniband/sw/rxe/rxe_net.c | 45 +++++++++++++++++++++++++++--
> > 1 file changed, 43 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> > index 8fd03ae20efc..6ee984b7793b 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_net.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> > @@ -368,6 +368,25 @@ static void prepare_ipv6_hdr(struct dst_entry *dst, struct sk_buff *skb,
> > ip6h->payload_len = htons(skb->len - sizeof(*ip6h));
> > }
> >
> > +static inline bool same_rxe4(struct net_device *ndev, struct in_addr *daddr)
> > +{
> > + struct in_device *in_dev;
> > + bool same_rxe = false;
> > +
> > + in_dev = in_dev_get(ndev);
> > + if (!in_dev)
> > + return false;
> > +
> > + for_ifa(in_dev)
> > + if (!memcmp(&ifa->ifa_address, daddr, sizeof(*daddr))) {
> > + same_rxe = true;
> > + break;
> > + }
> > + endfor_ifa(in_dev);
> > + in_dev_put(in_dev);
> > + return same_rxe;
> > +}
> > +
> > static int prepare4(struct rxe_pkt_info *pkt, struct sk_buff *skb,
> > struct rxe_av *av)
> > {
> > @@ -384,7 +403,7 @@ static int prepare4(struct rxe_pkt_info *pkt, struct sk_buff *skb,
> > return -EHOSTUNREACH;
> > }
> >
> > - if (!memcmp(saddr, daddr, sizeof(*daddr)))
> > + if (same_rxe4(skb->dev, daddr))
> > pkt->mask |= RXE_LOOPBACK_MASK;
> >
> > prepare_udp_hdr(skb, cpu_to_be16(qp->src_port),
> > @@ -397,6 +416,28 @@ static int prepare4(struct rxe_pkt_info *pkt, struct sk_buff *skb,
> > return 0;
> > }
> >
> > +static inline bool same_rxe6(struct net_device *ndev, struct in6_addr *daddr)
> > +{
> > + struct inet6_dev *in6_dev;
> > + struct inet6_ifaddr *ifp;
> > + bool same_rxe = false;
> > +
> > + in6_dev = in6_dev_get(ndev);
> > + if (!in6_dev)
> > + return false;
> > +
> > + read_lock_bh(&in6_dev->lock);
>
> To me at first glance it is unclear why lock is not needed when traveling
> the v4 list. Suggesting to comment on that.
>
> > + list_for_each_entry(ifp, &in6_dev->addr_list, if_list) {
> > + if (!memcmp(&ifp->addr, daddr, sizeof(*daddr))) {
> > + same_rxe = true;
> > + break;
> > + }
> > + }
> > + read_unlock_bh(&in6_dev->lock);
> > + in6_dev_put(in6_dev);
> > + return same_rxe;
> > +}
> > +
> > static int prepare6(struct rxe_pkt_info *pkt, struct sk_buff *skb,
> > struct rxe_av *av)
> > {
> > @@ -411,7 +452,7 @@ static int prepare6(struct rxe_pkt_info *pkt, struct sk_buff *skb,
> > return -EHOSTUNREACH;
> > }
> >
> > - if (!memcmp(saddr, daddr, sizeof(*daddr)))
> > + if (same_rxe6(skb->dev, daddr))
> > pkt->mask |= RXE_LOOPBACK_MASK;
>
> Thanks Kamal!
>
> Reviewed-by: Yuval Shaia <yuval.shaia@xxxxxxxxxx>
>
> Will give it a short test today.
Tested both RC and UD as well as with qemu guest with pvrdma device.
Tested-by: Yuval Shaia <yuval.shaia@xxxxxxxxxx>
(btw, nice to finally move to 5.0 -:))
>
> >
> > prepare_udp_hdr(skb, cpu_to_be16(qp->src_port),
> > --
> > 2.20.1
> >
