| Message ID | 20201008212753.265249-1-rpearson@hpe.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [for-next,v2] rdma_rxe: fix bug rejecting multicast packets | expand |
On Thu, Oct 08, 2020 at 04:27:53PM -0500, Bob Pearson wrote: > - Fix a bug in rxe_rcv that causes all multicast packets to be > dropped. Currently rxe_match_dgid is called for each packet > to verify that the destination IP address matches one of the > entries in the port source GID table. This is incorrect for > IP multicast addresses since they do not appear in the GID table. > - Add code to detect multicast addresses. > - Change function name to rxe_chk_dgid which is clearer. > > Signed-off-by: Bob Pearson <rpearson@hpe.com> > --- > drivers/infiniband/sw/rxe/rxe_recv.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) Looks right, was there 4 patches? I only see three Applied to for-next Thanks, Jason
On 10/9/2020 5:27 AM, Bob Pearson wrote: > - Fix a bug in rxe_rcv that causes all multicast packets to be > dropped. Currently rxe_match_dgid is called for each packet > to verify that the destination IP address matches one of the > entries in the port source GID table. This is incorrect for > IP multicast addresses since they do not appear in the GID table. > - Add code to detect multicast addresses. > - Change function name to rxe_chk_dgid which is clearer. > > Signed-off-by: Bob Pearson <rpearson@hpe.com> > --- > drivers/infiniband/sw/rxe/rxe_recv.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c > index a3eed4da1540..b6fee61b2aee 100644 > --- a/drivers/infiniband/sw/rxe/rxe_recv.c > +++ b/drivers/infiniband/sw/rxe/rxe_recv.c > @@ -280,7 +280,17 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb) > kfree_skb(skb); > } > > -static int rxe_match_dgid(struct rxe_dev *rxe, struct sk_buff *skb) > +/** > + * rxe_chk_dgid - validate destination IP address > + * @rxe: rxe device that received packet > + * @skb: the received packet buffer > + * > + * Accept any loopback packets About loopback packets, will rdma_find_gid_by_port return correct value? In my tests, to loopback packets, sometimes rdma_find_gid_by_port return incorrect value. Then the packets will be freed. Zhu Yanjun > + * Extract IP address from packet and > + * Accept if multicast packet > + * Accept if matches an SGID table entry > + */ > +static int rxe_chk_dgid(struct rxe_dev *rxe, struct sk_buff *skb) > { > struct rxe_pkt_info *pkt = SKB_TO_PKT(skb); > const struct ib_gid_attr *gid_attr; > @@ -298,6 +308,9 @@ static int rxe_match_dgid(struct rxe_dev *rxe, struct sk_buff *skb) > pdgid = (union ib_gid *)&ipv6_hdr(skb)->daddr; > } > > + if (rdma_is_multicast_addr((struct in6_addr *)pdgid)) > + return 0; > + > gid_attr = rdma_find_gid_by_port(&rxe->ib_dev, pdgid, > IB_GID_TYPE_ROCE_UDP_ENCAP, > 1, skb->dev); > @@ -322,8 +335,8 @@ void rxe_rcv(struct sk_buff *skb) > if (unlikely(skb->len < pkt->offset + RXE_BTH_BYTES)) > goto drop; > > - if (rxe_match_dgid(rxe, skb) < 0) { > - pr_warn_ratelimited("failed matching dgid\n"); > + if (rxe_chk_dgid(rxe, skb) < 0) { > + pr_warn_ratelimited("failed checking dgid\n"); > goto drop; > } >
On Fri, Oct 09, 2020 at 11:23:31PM +0800, Zhu Yanjun wrote: > On 10/9/2020 5:27 AM, Bob Pearson wrote: > > - Fix a bug in rxe_rcv that causes all multicast packets to be > > dropped. Currently rxe_match_dgid is called for each packet > > to verify that the destination IP address matches one of the > > entries in the port source GID table. This is incorrect for > > IP multicast addresses since they do not appear in the GID table. > > - Add code to detect multicast addresses. > > - Change function name to rxe_chk_dgid which is clearer. > > > > Signed-off-by: Bob Pearson <rpearson@hpe.com> > > drivers/infiniband/sw/rxe/rxe_recv.c | 19 ++++++++++++++++--- > > 1 file changed, 16 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c > > index a3eed4da1540..b6fee61b2aee 100644 > > +++ b/drivers/infiniband/sw/rxe/rxe_recv.c > > @@ -280,7 +280,17 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb) > > kfree_skb(skb); > > } > > -static int rxe_match_dgid(struct rxe_dev *rxe, struct sk_buff *skb) > > +/** > > + * rxe_chk_dgid - validate destination IP address > > + * @rxe: rxe device that received packet > > + * @skb: the received packet buffer > > + * > > + * Accept any loopback packets > > About loopback packets, will rdma_find_gid_by_port return correct value? I don't think you can use 127.0.0.0 with the RDMA devices, at least not on the wire. The CM has special code to swap it out with a real device address Jason
On 10/9/20 10:28 AM, Jason Gunthorpe wrote: > On Fri, Oct 09, 2020 at 11:23:31PM +0800, Zhu Yanjun wrote: >> On 10/9/2020 5:27 AM, Bob Pearson wrote: >>> - Fix a bug in rxe_rcv that causes all multicast packets to be >>> dropped. Currently rxe_match_dgid is called for each packet >>> to verify that the destination IP address matches one of the >>> entries in the port source GID table. This is incorrect for >>> IP multicast addresses since they do not appear in the GID table. >>> - Add code to detect multicast addresses. >>> - Change function name to rxe_chk_dgid which is clearer. >>> >>> Signed-off-by: Bob Pearson <rpearson@hpe.com> >>> drivers/infiniband/sw/rxe/rxe_recv.c | 19 ++++++++++++++++--- >>> 1 file changed, 16 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c >>> index a3eed4da1540..b6fee61b2aee 100644 >>> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c >>> @@ -280,7 +280,17 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb) >>> kfree_skb(skb); >>> } >>> -static int rxe_match_dgid(struct rxe_dev *rxe, struct sk_buff *skb) >>> +/** >>> + * rxe_chk_dgid - validate destination IP address >>> + * @rxe: rxe device that received packet >>> + * @skb: the received packet buffer >>> + * >>> + * Accept any loopback packets >> >> About loopback packets, will rdma_find_gid_by_port return correct value? I didn't touch that but the RXE_LOOPBACK test comes before the call to rdma_find_gid_by_port so it should never get called for loopback packets. > > I don't think you can use 127.0.0.0 with the RDMA devices, at least > not on the wire. The CM has special code to swap it out with a real > device address The following does work: $ ib_send_bw -d rxe_0 (in window A) $ ib_send_bw -d rxe_0 127.0.0.1 (in window B) This uses the LOOPBACK path and just hands the skb from sender to receiver. It never touches the IP stack. I have never been able to get this to work: $ ib_send_bw -d rxe_1 (at 10.0.0.1 in window A) $ ib_send_bw -d rxe_2 10.0.0.1 (at 10.0.0.2 in window B) If it did work I could test the full path. > > Jason >
On 10/10/2020 1:18 AM, Bob Pearson wrote: > On 10/9/20 10:28 AM, Jason Gunthorpe wrote: >> On Fri, Oct 09, 2020 at 11:23:31PM +0800, Zhu Yanjun wrote: >>> On 10/9/2020 5:27 AM, Bob Pearson wrote: >>>> - Fix a bug in rxe_rcv that causes all multicast packets to be >>>> dropped. Currently rxe_match_dgid is called for each packet >>>> to verify that the destination IP address matches one of the >>>> entries in the port source GID table. This is incorrect for >>>> IP multicast addresses since they do not appear in the GID table. >>>> - Add code to detect multicast addresses. >>>> - Change function name to rxe_chk_dgid which is clearer. >>>> >>>> Signed-off-by: Bob Pearson <rpearson@hpe.com> >>>> drivers/infiniband/sw/rxe/rxe_recv.c | 19 ++++++++++++++++--- >>>> 1 file changed, 16 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c >>>> index a3eed4da1540..b6fee61b2aee 100644 >>>> +++ b/drivers/infiniband/sw/rxe/rxe_recv.c >>>> @@ -280,7 +280,17 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb) >>>> kfree_skb(skb); >>>> } >>>> -static int rxe_match_dgid(struct rxe_dev *rxe, struct sk_buff *skb) >>>> +/** >>>> + * rxe_chk_dgid - validate destination IP address >>>> + * @rxe: rxe device that received packet >>>> + * @skb: the received packet buffer >>>> + * >>>> + * Accept any loopback packets >>> About loopback packets, will rdma_find_gid_by_port return correct value? > I didn't touch that but the RXE_LOOPBACK test comes before the call to rdma_find_gid_by_port > so it should never get called for loopback packets. I confronted the loopback problem with rdma-core tests. And I made a patch to fix it. If the following commit exists, this problem will not occur. commit 5c99274be8864519328aa74bc550ba410095bc1c Author: Zhu Yanjun <yanjunz@mellanox.com> Date: Tue Jun 30 15:36:05 2020 +0300 RDMA/rxe: Skip dgid check in loopback mode In the loopback tests, the following call trace occurs. Call Trace: __rxe_do_task+0x1a/0x30 [rdma_rxe] rxe_qp_destroy+0x61/0xa0 [rdma_rxe] rxe_destroy_qp+0x20/0x60 [rdma_rxe] ib_destroy_qp_user+0xcc/0x220 [ib_core] uverbs_free_qp+0x3c/0xc0 [ib_uverbs] destroy_hw_idr_uobject+0x24/0x70 [ib_uverbs] uverbs_destroy_uobject+0x43/0x1b0 [ib_uverbs] uobj_destroy+0x41/0x70 [ib_uverbs] __uobj_get_destroy+0x39/0x70 [ib_uverbs] ib_uverbs_destroy_qp+0x88/0xc0 [ib_uverbs] ib_uverbs_handler_UVERBS_METHOD_INVOKE_WRITE+0xb9/0xf0 [ib_uverbs] ib_uverbs_cmd_verbs+0xb16/0xc30 [ib_uverbs] The root cause is that the actual RDMA connection is not created in the loopback tests and the rxe_match_dgid will fail randomly. To fix this call trace which appear in the loopback tests, skip check of the dgid. Fixes: 8700e3e7c485 ("Soft RoCE driver") Link: https://lore.kernel.org/r/20200630123605.446959-1-leon@kernel.org Signed-off-by: Zhu Yanjun <yanjunz@mellanox.com> Signed-off-by: Leon Romanovsky <leonro@mellanox.com> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> >> I don't think you can use 127.0.0.0 with the RDMA devices, at least >> not on the wire. The CM has special code to swap it out with a real >> device address > The following does work: > > $ ib_send_bw -d rxe_0 (in window A) $ ib_send_bw -d rxe_0 127.0.0.1 (in window B) > > This uses the LOOPBACK path and just hands the skb from sender to receiver. It never touches the IP stack. > > I have never been able to get this to work: > > $ ib_send_bw -d rxe_1 (at 10.0.0.1 in window A) $ ib_send_bw -d rxe_2 10.0.0.1 (at 10.0.0.2 in window B) > > If it did work I could test the full path. >> Jason >>
diff --git a/drivers/infiniband/sw/rxe/rxe_recv.c b/drivers/infiniband/sw/rxe/rxe_recv.c index a3eed4da1540..b6fee61b2aee 100644 --- a/drivers/infiniband/sw/rxe/rxe_recv.c +++ b/drivers/infiniband/sw/rxe/rxe_recv.c @@ -280,7 +280,17 @@ static void rxe_rcv_mcast_pkt(struct rxe_dev *rxe, struct sk_buff *skb) kfree_skb(skb); } -static int rxe_match_dgid(struct rxe_dev *rxe, struct sk_buff *skb) +/** + * rxe_chk_dgid - validate destination IP address + * @rxe: rxe device that received packet + * @skb: the received packet buffer + * + * Accept any loopback packets + * Extract IP address from packet and + * Accept if multicast packet + * Accept if matches an SGID table entry + */ +static int rxe_chk_dgid(struct rxe_dev *rxe, struct sk_buff *skb) { struct rxe_pkt_info *pkt = SKB_TO_PKT(skb); const struct ib_gid_attr *gid_attr; @@ -298,6 +308,9 @@ static int rxe_match_dgid(struct rxe_dev *rxe, struct sk_buff *skb) pdgid = (union ib_gid *)&ipv6_hdr(skb)->daddr; } + if (rdma_is_multicast_addr((struct in6_addr *)pdgid)) + return 0; + gid_attr = rdma_find_gid_by_port(&rxe->ib_dev, pdgid, IB_GID_TYPE_ROCE_UDP_ENCAP, 1, skb->dev); @@ -322,8 +335,8 @@ void rxe_rcv(struct sk_buff *skb) if (unlikely(skb->len < pkt->offset + RXE_BTH_BYTES)) goto drop; - if (rxe_match_dgid(rxe, skb) < 0) { - pr_warn_ratelimited("failed matching dgid\n"); + if (rxe_chk_dgid(rxe, skb) < 0) { + pr_warn_ratelimited("failed checking dgid\n"); goto drop; }
- Fix a bug in rxe_rcv that causes all multicast packets to be dropped. Currently rxe_match_dgid is called for each packet to verify that the destination IP address matches one of the entries in the port source GID table. This is incorrect for IP multicast addresses since they do not appear in the GID table. - Add code to detect multicast addresses. - Change function name to rxe_chk_dgid which is clearer. Signed-off-by: Bob Pearson <rpearson@hpe.com> --- drivers/infiniband/sw/rxe/rxe_recv.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)