Re: [PATCH rdma-next 2/2] RDMA/rxe: Improve loopback marking

[Yanjun Zhu <yanjun.zhu@xxxxxxxxxx>]

On 2019/1/30 11:09, Jason Gunthorpe wrote:
> On Wed, Jan 30, 2019 at 10:48:10AM +0800, Yanjun Zhu wrote:
> > On 2019/1/29 18:08, Kamal Heib wrote:
> > > Currently a packet is marked for loopback only if the source and
> > > destination addresses equals. This is not enough when multiple gids are
> > > present in rxe device's gid table and the traffic is from one gid to
> > > another. Fix it by marking the packet for loopback if the destination
> > > MAC address is equal to the source MAC address.
> > >
> > > Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx>
> > >    drivers/infiniband/sw/rxe/rxe_av.c  | 1 +
> > >    drivers/infiniband/sw/rxe/rxe_net.c | 9 +++------
> > >    include/uapi/rdma/rdma_user_rxe.h   | 3 +--
> > >    3 files changed, 5 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/sw/rxe/rxe_av.c b/drivers/infiniband/sw/rxe/rxe_av.c
> > > index 27a7dec18874..81ee756c19b8 100644
> > > +++ b/drivers/infiniband/sw/rxe/rxe_av.c
> > > @@ -38,6 +38,7 @@ void rxe_init_av(struct rdma_ah_attr *attr, struct rxe_av *av)
> > >    {
> > >    	rxe_av_from_attr(rdma_ah_get_port_num(attr), av, attr);
> > >    	rxe_av_fill_ip_info(av, attr);
> > > +	memcpy(av->dmac, attr->roce.dmac, ETH_ALEN);
> > >    }
> > >    int rxe_av_chk_attr(struct rxe_dev *rxe, struct rdma_ah_attr *attr)
> > > diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> > > index 8fd03ae20efc..87dfb16744cc 100644
> > > +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> > > @@ -384,9 +384,6 @@ static int prepare4(struct rxe_pkt_info *pkt, struct sk_buff *skb,
> > >    		return -EHOSTUNREACH;
> > >    	}
> > > -	if (!memcmp(saddr, daddr, sizeof(*daddr)))
> > > -		pkt->mask |= RXE_LOOPBACK_MASK;
> > > -
> > >    	prepare_udp_hdr(skb, cpu_to_be16(qp->src_port),
> > >    			cpu_to_be16(ROCE_V2_UDP_DPORT));
> > > @@ -411,9 +408,6 @@ static int prepare6(struct rxe_pkt_info *pkt, struct sk_buff *skb,
> > >    		return -EHOSTUNREACH;
> > >    	}
> > > -	if (!memcmp(saddr, daddr, sizeof(*daddr)))
> > > -		pkt->mask |= RXE_LOOPBACK_MASK;
> > > -
> > >    	prepare_udp_hdr(skb, cpu_to_be16(qp->src_port),
> > >    			cpu_to_be16(ROCE_V2_UDP_DPORT));
> > > @@ -437,6 +431,9 @@ int rxe_prepare(struct rxe_pkt_info *pkt, struct sk_buff *skb, u32 *crc)
> > >    	*crc = rxe_icrc_hdr(pkt, skb);
> > > +	if (ether_addr_equal(skb->dev->dev_addr, av->dmac))
> > > +		pkt->mask |= RXE_LOOPBACK_MASK;
> > > +
> > Sorry. A problem.
> >
> > This rxe device can work on bonding. When the active slave interface is
> > changed, it is possible that the mac address of bonding will also be
> > changed.
> That isn't bonding, that is some kind of failover configuration - and
> I doubt rxe has the necessary support for working with bonding or
> failover, or responding to dynamic changes in the LLADDrR..

I just doubt whether rxe can work well or not when the mac address is 
changed.

If rxe does not support this kind of changed mac address, my doubt can 
be ignored.

Thanks,

Zhu Yanjun

> Jason