| Message ID | 20190129100850.18840-3-kamalheib1@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 668aa15b5bf87f156ec805cb7348c785c56b82ab |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | RDMA/rxe: Fix loopback marking | expand |
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@gmail.com> > --- > 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 > --- a/drivers/infiniband/sw/rxe/rxe_av.c > +++ 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 > --- a/drivers/infiniband/sw/rxe/rxe_net.c > +++ 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. In this scenario, the above still can work well? Zhu Yanjun > return err; > } > > diff --git a/include/uapi/rdma/rdma_user_rxe.h b/include/uapi/rdma/rdma_user_rxe.h > index 44ef6a3b7afc..aae2e696bb38 100644 > --- a/include/uapi/rdma/rdma_user_rxe.h > +++ b/include/uapi/rdma/rdma_user_rxe.h > @@ -58,8 +58,7 @@ struct rxe_global_route { > struct rxe_av { > __u8 port_num; > __u8 network_type; > - __u16 reserved1; > - __u32 reserved2; > + __u8 dmac[6]; > struct rxe_global_route grh; > union { > struct sockaddr_in _sockaddr_in;
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@gmail.com> > > 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.. Jason
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@gmail.com> >>> 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 >
On Wed, Jan 30, 2019 at 11:24:24AM +0800, Yanjun Zhu wrote: > > 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@gmail.com> > > > > 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 Hi Yanjun, I'm not following. So you are talking about the case where the rxe's Ethernet device changes its MAC, right? So, in that case, what do you suggest the device will do with all the packets in the pipe directed to the old MAC? As far as my understanding those would be dropped anyway, aren't they? Yuval > > > > > Jason > >
On Tue, Jan 29, 2019 at 12:08:50PM +0200, 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@gmail.com> > --- > 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 > --- a/drivers/infiniband/sw/rxe/rxe_av.c > +++ 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); This is the missing part that i was worried about, i was not sure that dmac is set at the time that prepare? was issued. > } > > 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 > --- a/drivers/infiniband/sw/rxe/rxe_net.c > +++ 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; > + > return err; > } > > diff --git a/include/uapi/rdma/rdma_user_rxe.h b/include/uapi/rdma/rdma_user_rxe.h > index 44ef6a3b7afc..aae2e696bb38 100644 > --- a/include/uapi/rdma/rdma_user_rxe.h > +++ b/include/uapi/rdma/rdma_user_rxe.h > @@ -58,8 +58,7 @@ struct rxe_global_route { > struct rxe_av { > __u8 port_num; > __u8 network_type; > - __u16 reserved1; > - __u32 reserved2; > + __u8 dmac[6]; We are so lucky!! > struct rxe_global_route grh; > union { > struct sockaddr_in _sockaddr_in; Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com> and also: Tested-by: Yuval Shaia <yuval.shaia@oracle.com> > -- > 2.20.1 >
在 2019/1/30 20:55, Yuval Shaia 写道: > On Wed, Jan 30, 2019 at 11:24:24AM +0800, Yanjun Zhu wrote: >> 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@gmail.com> >>>>> 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 > Hi Yanjun, > I'm not following. > So you are talking about the case where the rxe's Ethernet device changes > its MAC, right? > So, in that case, what do you suggest the device will do with all the > packets in the pipe directed to the old MAC? As far as my understanding > those would be dropped anyway, aren't they? Sorry. I do not know whether rxe can work well or not. As to how to do with all the packets, I have no idea. Perhaps the rxe can work well, it can accept these packets. We will need the tests to verify it.:-) Zhu Yanjun > > Yuval > >>> Jason >>>
On Wed, Jan 30, 2019 at 09:14:09PM +0800, Yanjun Zhu wrote: > > 在 2019/1/30 20:55, Yuval Shaia 写道: > > On Wed, Jan 30, 2019 at 11:24:24AM +0800, Yanjun Zhu wrote: > > > 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@gmail.com> > > > > > > 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 > > Hi Yanjun, > > I'm not following. > > So you are talking about the case where the rxe's Ethernet device changes > > its MAC, right? > > So, in that case, what do you suggest the device will do with all the > > packets in the pipe directed to the old MAC? As far as my understanding > > those would be dropped anyway, aren't they? > > Sorry. I do not know whether rxe can work well or not. As to how to do with > all > > the packets, I have no idea. Perhaps the rxe can work well, it can accept > these packets. > > We will need the tests to verify it.:-) > > Zhu Yanjun Per your suggestion i did the test and found that as long as there is no traffic at the time of the MAC change we are fine but if messages are in the queue to be sent and then MAC change happen then they will be dropped. > > > > > Yuval > > > > > > Jason > > > >
diff --git a/drivers/infiniband/sw/rxe/rxe_av.c b/drivers/infiniband/sw/rxe/rxe_av.c index 27a7dec18874..81ee756c19b8 100644 --- a/drivers/infiniband/sw/rxe/rxe_av.c +++ 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 --- a/drivers/infiniband/sw/rxe/rxe_net.c +++ 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; + return err; } diff --git a/include/uapi/rdma/rdma_user_rxe.h b/include/uapi/rdma/rdma_user_rxe.h index 44ef6a3b7afc..aae2e696bb38 100644 --- a/include/uapi/rdma/rdma_user_rxe.h +++ b/include/uapi/rdma/rdma_user_rxe.h @@ -58,8 +58,7 @@ struct rxe_global_route { struct rxe_av { __u8 port_num; __u8 network_type; - __u16 reserved1; - __u32 reserved2; + __u8 dmac[6]; struct rxe_global_route grh; union { struct sockaddr_in _sockaddr_in;
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@gmail.com> --- 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(-)