* [PATCH 1/1] RDMA/irdma: Make the source udp port vary
@ 2021-12-14 5:42 yanjun.zhu
2021-12-14 17:27 ` Tom Talpey
2021-12-17 2:02 ` Saleem, Shiraz
0 siblings, 2 replies; 7+ messages in thread
From: yanjun.zhu @ 2021-12-14 5:42 UTC (permalink / raw)
To: mustafa.ismail, shiraz.saleem, dledford, jgg, linux-rdma, yanjun.zhu
From: Zhu Yanjun <yanjun.zhu@linux.dev>
Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
get the source udp port number for a QP based on the local QPN. This
provides a better spread of traffic across NIC RX queues. The method in
the commit d3c04a3a6870 ("IB/rxe: vary the source udp port for receive
scaling") is stable. So it is also adopted in this commit.
Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
---
drivers/infiniband/hw/irdma/verbs.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
index 102dc9342f2a..2697b40a539e 100644
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ b/drivers/infiniband/hw/irdma/verbs.c
@@ -690,6 +690,11 @@ static int irdma_cqp_create_qp_cmd(struct irdma_qp *iwqp)
return status ? -ENOMEM : 0;
}
+static inline u16 irdma_get_src_port(struct irdma_qp *iwqp)
+{
+ return 0xc000 + (hash_32_generic(iwqp->ibqp.qp_num, 14) & 0x3fff);
+}
+
static void irdma_roce_fill_and_set_qpctx_info(struct irdma_qp *iwqp,
struct irdma_qp_host_ctx_info *ctx_info)
{
@@ -703,7 +708,7 @@ static void irdma_roce_fill_and_set_qpctx_info(struct irdma_qp *iwqp,
udp_info->cwnd = iwdev->roce_cwnd;
udp_info->rexmit_thresh = 2;
udp_info->rnr_nak_thresh = 2;
- udp_info->src_port = 0xc000;
+ udp_info->src_port = irdma_get_src_port(iwqp);
udp_info->dst_port = ROCE_V2_UDP_DPORT;
roce_info = &iwqp->roce_info;
ether_addr_copy(roce_info->mac_addr, iwdev->netdev->dev_addr);
--
2.27.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] RDMA/irdma: Make the source udp port vary
2021-12-14 5:42 [PATCH 1/1] RDMA/irdma: Make the source udp port vary yanjun.zhu
@ 2021-12-14 17:27 ` Tom Talpey
2021-12-14 17:29 ` Jason Gunthorpe
2021-12-17 2:02 ` Saleem, Shiraz
1 sibling, 1 reply; 7+ messages in thread
From: Tom Talpey @ 2021-12-14 17:27 UTC (permalink / raw)
To: yanjun.zhu, mustafa.ismail, shiraz.saleem, dledford, jgg, linux-rdma
On 12/14/2021 12:42 AM, yanjun.zhu@linux.dev wrote:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>
> Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
> get the source udp port number for a QP based on the local QPN. This
> provides a better spread of traffic across NIC RX queues. The method in
> the commit d3c04a3a6870 ("IB/rxe: vary the source udp port for receive
> scaling") is stable. So it is also adopted in this commit.
>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> ---
> drivers/infiniband/hw/irdma/verbs.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> index 102dc9342f2a..2697b40a539e 100644
> --- a/drivers/infiniband/hw/irdma/verbs.c
> +++ b/drivers/infiniband/hw/irdma/verbs.c
> @@ -690,6 +690,11 @@ static int irdma_cqp_create_qp_cmd(struct irdma_qp *iwqp)
> return status ? -ENOMEM : 0;
> }
>
> +static inline u16 irdma_get_src_port(struct irdma_qp *iwqp)
> +{
> + return 0xc000 + (hash_32_generic(iwqp->ibqp.qp_num, 14) & 0x3fff);
> +}
How do you ensure the resulting port number is not already in use?
Tom.
> +
> static void irdma_roce_fill_and_set_qpctx_info(struct irdma_qp *iwqp,
> struct irdma_qp_host_ctx_info *ctx_info)
> {
> @@ -703,7 +708,7 @@ static void irdma_roce_fill_and_set_qpctx_info(struct irdma_qp *iwqp,
> udp_info->cwnd = iwdev->roce_cwnd;
> udp_info->rexmit_thresh = 2;
> udp_info->rnr_nak_thresh = 2;
> - udp_info->src_port = 0xc000;
> + udp_info->src_port = irdma_get_src_port(iwqp);
> udp_info->dst_port = ROCE_V2_UDP_DPORT;
> roce_info = &iwqp->roce_info;
> ether_addr_copy(roce_info->mac_addr, iwdev->netdev->dev_addr);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] RDMA/irdma: Make the source udp port vary
2021-12-14 17:27 ` Tom Talpey
@ 2021-12-14 17:29 ` Jason Gunthorpe
2021-12-14 18:09 ` Tom Talpey
0 siblings, 1 reply; 7+ messages in thread
From: Jason Gunthorpe @ 2021-12-14 17:29 UTC (permalink / raw)
To: Tom Talpey
Cc: yanjun.zhu, mustafa.ismail, shiraz.saleem, dledford, linux-rdma
On Tue, Dec 14, 2021 at 12:27:24PM -0500, Tom Talpey wrote:
> On 12/14/2021 12:42 AM, yanjun.zhu@linux.dev wrote:
> > From: Zhu Yanjun <yanjun.zhu@linux.dev>
> >
> > Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
> > get the source udp port number for a QP based on the local QPN. This
> > provides a better spread of traffic across NIC RX queues. The method in
> > the commit d3c04a3a6870 ("IB/rxe: vary the source udp port for receive
> > scaling") is stable. So it is also adopted in this commit.
> >
> > Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> > drivers/infiniband/hw/irdma/verbs.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> > index 102dc9342f2a..2697b40a539e 100644
> > +++ b/drivers/infiniband/hw/irdma/verbs.c
> > @@ -690,6 +690,11 @@ static int irdma_cqp_create_qp_cmd(struct irdma_qp *iwqp)
> > return status ? -ENOMEM : 0;
> > }
> > +static inline u16 irdma_get_src_port(struct irdma_qp *iwqp)
> > +{
> > + return 0xc000 + (hash_32_generic(iwqp->ibqp.qp_num, 14) & 0x3fff);
> > +}
>
> How do you ensure the resulting port number is not already in use?
It doesn't matter, it is never used by anything, the receiver captures
all data with the roce dport and ignores the sport
Jason
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] RDMA/irdma: Make the source udp port vary
2021-12-14 17:29 ` Jason Gunthorpe
@ 2021-12-14 18:09 ` Tom Talpey
2021-12-14 18:50 ` Jason Gunthorpe
0 siblings, 1 reply; 7+ messages in thread
From: Tom Talpey @ 2021-12-14 18:09 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: yanjun.zhu, mustafa.ismail, shiraz.saleem, dledford, linux-rdma
On 12/14/2021 12:29 PM, Jason Gunthorpe wrote:
> On Tue, Dec 14, 2021 at 12:27:24PM -0500, Tom Talpey wrote:
>> On 12/14/2021 12:42 AM, yanjun.zhu@linux.dev wrote:
>>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>>
>>> Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
>>> get the source udp port number for a QP based on the local QPN. This
>>> provides a better spread of traffic across NIC RX queues. The method in
>>> the commit d3c04a3a6870 ("IB/rxe: vary the source udp port for receive
>>> scaling") is stable. So it is also adopted in this commit.
>>>
>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
>>> drivers/infiniband/hw/irdma/verbs.c | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
>>> index 102dc9342f2a..2697b40a539e 100644
>>> +++ b/drivers/infiniband/hw/irdma/verbs.c
>>> @@ -690,6 +690,11 @@ static int irdma_cqp_create_qp_cmd(struct irdma_qp *iwqp)
>>> return status ? -ENOMEM : 0;
>>> }
>>> +static inline u16 irdma_get_src_port(struct irdma_qp *iwqp)
>>> +{
>>> + return 0xc000 + (hash_32_generic(iwqp->ibqp.qp_num, 14) & 0x3fff);
>>> +}
>>
>> How do you ensure the resulting port number is not already in use?
>
> It doesn't matter, it is never used by anything, the receiver captures
> all data with the roce dport and ignores the sport
It still violates core networking addressing principles, and will
mightily confuse a network capture that's filtering on source ports.
Firewalls, ICMP, and similar fabric behaviors may also interfere.
SoftRoCE is forced to register/reserve the source port, isn't it?
Tom.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] RDMA/irdma: Make the source udp port vary
2021-12-14 18:09 ` Tom Talpey
@ 2021-12-14 18:50 ` Jason Gunthorpe
0 siblings, 0 replies; 7+ messages in thread
From: Jason Gunthorpe @ 2021-12-14 18:50 UTC (permalink / raw)
To: Tom Talpey
Cc: yanjun.zhu, mustafa.ismail, shiraz.saleem, dledford, linux-rdma
On Tue, Dec 14, 2021 at 01:09:01PM -0500, Tom Talpey wrote:
> On 12/14/2021 12:29 PM, Jason Gunthorpe wrote:
> > On Tue, Dec 14, 2021 at 12:27:24PM -0500, Tom Talpey wrote:
> > > On 12/14/2021 12:42 AM, yanjun.zhu@linux.dev wrote:
> > > > From: Zhu Yanjun <yanjun.zhu@linux.dev>
> > > >
> > > > Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
> > > > get the source udp port number for a QP based on the local QPN. This
> > > > provides a better spread of traffic across NIC RX queues. The method in
> > > > the commit d3c04a3a6870 ("IB/rxe: vary the source udp port for receive
> > > > scaling") is stable. So it is also adopted in this commit.
> > > >
> > > > Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> > > > drivers/infiniband/hw/irdma/verbs.c | 7 ++++++-
> > > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> > > > index 102dc9342f2a..2697b40a539e 100644
> > > > +++ b/drivers/infiniband/hw/irdma/verbs.c
> > > > @@ -690,6 +690,11 @@ static int irdma_cqp_create_qp_cmd(struct irdma_qp *iwqp)
> > > > return status ? -ENOMEM : 0;
> > > > }
> > > > +static inline u16 irdma_get_src_port(struct irdma_qp *iwqp)
> > > > +{
> > > > + return 0xc000 + (hash_32_generic(iwqp->ibqp.qp_num, 14) & 0x3fff);
> > > > +}
> > >
> > > How do you ensure the resulting port number is not already in use?
> >
> > It doesn't matter, it is never used by anything, the receiver captures
> > all data with the roce dport and ignores the sport
>
> It still violates core networking addressing principles, and will
> mightily confuse a network capture that's filtering on source ports.
> Firewalls, ICMP, and similar fabric behaviors may also interfere.
Maybe, but most of that stuff doesn't work with roce anyhow.
> SoftRoCE is forced to register/reserve the source port, isn't it?
Logically it has to register the dest port, it receives from any
source port.
Due to the way the netstack works softroce can't do this trick either,
IIRC.
Jason
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/1] RDMA/irdma: Make the source udp port vary
2021-12-14 5:42 [PATCH 1/1] RDMA/irdma: Make the source udp port vary yanjun.zhu
2021-12-14 17:27 ` Tom Talpey
@ 2021-12-17 2:02 ` Saleem, Shiraz
2021-12-18 2:29 ` Yanjun Zhu
1 sibling, 1 reply; 7+ messages in thread
From: Saleem, Shiraz @ 2021-12-17 2:02 UTC (permalink / raw)
To: yanjun.zhu, Ismail, Mustafa, dledford, jgg, linux-rdma
> Subject: [PATCH 1/1] RDMA/irdma: Make the source udp port vary
>
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>
> Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
> get the source udp port number for a QP based on the local QPN. This provides a
> better spread of traffic across NIC RX queues. The method in the commit
> d3c04a3a6870 ("IB/rxe: vary the source udp port for receive
> scaling") is stable. So it is also adopted in this commit.
>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> ---
> drivers/infiniband/hw/irdma/verbs.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> index 102dc9342f2a..2697b40a539e 100644
> --- a/drivers/infiniband/hw/irdma/verbs.c
> +++ b/drivers/infiniband/hw/irdma/verbs.c
> @@ -690,6 +690,11 @@ static int irdma_cqp_create_qp_cmd(struct irdma_qp
> *iwqp)
> return status ? -ENOMEM : 0;
> }
>
> +static inline u16 irdma_get_src_port(struct irdma_qp *iwqp) {
> + return 0xc000 + (hash_32_generic(iwqp->ibqp.qp_num, 14) & 0x3fff); }
> +
There are core hash function helpers based on the grh.flow_label or lqpn/rqrpn that RoCEv2 drivers could use the to get the UDP src port?
https://elixir.bootlin.com/linux/v5.16-rc5/source/include/rdma/ib_verbs.h#L4719
Why don't we use them instead to set the udp_info->src_port in irdma_modify_qp_roce when the path address vector is provided?
Shiraz
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] RDMA/irdma: Make the source udp port vary
2021-12-17 2:02 ` Saleem, Shiraz
@ 2021-12-18 2:29 ` Yanjun Zhu
0 siblings, 0 replies; 7+ messages in thread
From: Yanjun Zhu @ 2021-12-18 2:29 UTC (permalink / raw)
To: Saleem, Shiraz, yanjun.zhu, Ismail, Mustafa, dledford, jgg, linux-rdma
在 2021/12/17 10:02, Saleem, Shiraz 写道:
>> Subject: [PATCH 1/1] RDMA/irdma: Make the source udp port vary
>>
>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>
>> Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
>> get the source udp port number for a QP based on the local QPN. This provides a
>> better spread of traffic across NIC RX queues. The method in the commit
>> d3c04a3a6870 ("IB/rxe: vary the source udp port for receive
>> scaling") is stable. So it is also adopted in this commit.
>>
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
>> ---
>> drivers/infiniband/hw/irdma/verbs.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
>> index 102dc9342f2a..2697b40a539e 100644
>> --- a/drivers/infiniband/hw/irdma/verbs.c
>> +++ b/drivers/infiniband/hw/irdma/verbs.c
>> @@ -690,6 +690,11 @@ static int irdma_cqp_create_qp_cmd(struct irdma_qp
>> *iwqp)
>> return status ? -ENOMEM : 0;
>> }
>>
>> +static inline u16 irdma_get_src_port(struct irdma_qp *iwqp) {
>> + return 0xc000 + (hash_32_generic(iwqp->ibqp.qp_num, 14) & 0x3fff); }
>> +
>
> There are core hash function helpers based on the grh.flow_label or lqpn/rqrpn that RoCEv2 drivers could use the to get the UDP src port?
>
> https://elixir.bootlin.com/linux/v5.16-rc5/source/include/rdma/ib_verbs.h#L4719
>
> Why don't we use them instead to set the udp_info->src_port in irdma_modify_qp_roce when the path address vector is provided?
Got it. I will send a new patch based on your suggestion.
Thanks.
Zhu Yanjun
>
> Shiraz
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-12-18 2:29 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-14 5:42 [PATCH 1/1] RDMA/irdma: Make the source udp port vary yanjun.zhu
2021-12-14 17:27 ` Tom Talpey
2021-12-14 17:29 ` Jason Gunthorpe
2021-12-14 18:09 ` Tom Talpey
2021-12-14 18:50 ` Jason Gunthorpe
2021-12-17 2:02 ` Saleem, Shiraz
2021-12-18 2:29 ` Yanjun Zhu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.