diff mbox

[4/4] IB/rxe: exchange the 2 udp ports

Message ID 1526133268-23926-5-git-send-email-yanjun.zhu@oracle.com (mailing list archive)
State Rejected
Headers show

Commit Message

Zhu Yanjun May 12, 2018, 1:54 p.m. UTC
When udp port 4791 is blocked, the udp port 4891 is used and vice versa.

CC: Srinivas Eeda <srinivas.eeda@oracle.com>
CC: Junxiao Bi <junxiao.bi@oracle.com>
Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
 drivers/infiniband/sw/rxe/rxe_net.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Tom Talpey May 12, 2018, 2:24 p.m. UTC | #1
On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
> When udp port 4791 is blocked, the udp port 4891 is used and vice versa.

Port 4891 is currently unassigned in the IANA registry. Do you intend
to request this? Strongly suggest that this not merge without such a
standard.

https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt


> 
> CC: Srinivas Eeda <srinivas.eeda@oracle.com>
> CC: Junxiao Bi <junxiao.bi@oracle.com>
> Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
> ---
>   drivers/infiniband/sw/rxe/rxe_net.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 29f6937..d61f2c5 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -276,6 +276,16 @@ static inline void prepare_udp_ports_spin(__be16 dport, __be16 sport)
>   	src_port = sport;
>   	spin_unlock(&port_lock);
>   }
> +
> +static inline void exchange_udp_port(void)
> +{
> +	if (dst_port == htons(ROCE_V2_UDP_DPORT))
> +		prepare_udp_ports_spin(htons(ROCE_V2_UDP_DUAL_DPORT),
> +				       htons(RXE_ROCE_V2_DUAL_SPORT));
> +	else
> +		prepare_udp_ports_spin(htons(ROCE_V2_UDP_DPORT),
> +				       htons(RXE_ROCE_V2_SPORT));
> +}
>   #endif
>   
>   static int rxe_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
> @@ -549,8 +559,7 @@ int rxe_send(struct rxe_pkt_info *pkt, struct sk_buff *skb)
>   
>   #ifdef CONFIG_RDMA_RXE_DUAL_PORT_MODULE
>   	if (unlikely(err != NET_XMIT_SUCCESS))
> -		prepare_udp_ports_spin(htons(ROCE_V2_UDP_DUAL_DPORT),
> -				       htons(RXE_ROCE_V2_DUAL_SPORT));
> +		exchange_udp_port();
>   #endif
>   
>   	if (unlikely(net_xmit_eval(err))) {
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky May 12, 2018, 2:55 p.m. UTC | #2
On Sat, May 12, 2018 at 10:24:03AM -0400, Tom Talpey wrote:
> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
> > When udp port 4791 is blocked, the udp port 4891 is used and vice versa.
>
> Port 4891 is currently unassigned in the IANA registry. Do you intend
> to request this? Strongly suggest that this not merge without such a
> standard.
>
> https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt
>
>

The whole idea looks to me like a hack, what will we do once the second
port blocked too? Will we introduce option to add more ports?

Thanks
Zhu Yanjun May 13, 2018, 1 a.m. UTC | #3
On 2018/5/12 22:24, Tom Talpey wrote:
> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>> When udp port 4791 is blocked, the udp port 4891 is used and vice versa.
>
> Port 4891 is currently unassigned in the IANA registry. Do you intend
> to request this? 
Yes. If this patch is merged, I will request it.

Zhu Yanjun
> Strongly suggest that this not merge without such a
> standard.
>
> https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt 
>
>
>
>>
>> CC: Srinivas Eeda <srinivas.eeda@oracle.com>
>> CC: Junxiao Bi <junxiao.bi@oracle.com>
>> Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
>> ---
>>   drivers/infiniband/sw/rxe/rxe_net.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c 
>> b/drivers/infiniband/sw/rxe/rxe_net.c
>> index 29f6937..d61f2c5 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
>> @@ -276,6 +276,16 @@ static inline void prepare_udp_ports_spin(__be16 
>> dport, __be16 sport)
>>       src_port = sport;
>>       spin_unlock(&port_lock);
>>   }
>> +
>> +static inline void exchange_udp_port(void)
>> +{
>> +    if (dst_port == htons(ROCE_V2_UDP_DPORT))
>> +        prepare_udp_ports_spin(htons(ROCE_V2_UDP_DUAL_DPORT),
>> +                       htons(RXE_ROCE_V2_DUAL_SPORT));
>> +    else
>> +        prepare_udp_ports_spin(htons(ROCE_V2_UDP_DPORT),
>> +                       htons(RXE_ROCE_V2_SPORT));
>> +}
>>   #endif
>>     static int rxe_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
>> @@ -549,8 +559,7 @@ int rxe_send(struct rxe_pkt_info *pkt, struct 
>> sk_buff *skb)
>>     #ifdef CONFIG_RDMA_RXE_DUAL_PORT_MODULE
>>       if (unlikely(err != NET_XMIT_SUCCESS))
>> -        prepare_udp_ports_spin(htons(ROCE_V2_UDP_DUAL_DPORT),
>> -                       htons(RXE_ROCE_V2_DUAL_SPORT));
>> +        exchange_udp_port();
>>   #endif
>>         if (unlikely(net_xmit_eval(err))) {
>>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhu Yanjun May 13, 2018, 1:05 a.m. UTC | #4
On 2018/5/12 22:55, Leon Romanovsky wrote:
> On Sat, May 12, 2018 at 10:24:03AM -0400, Tom Talpey wrote:
>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>> When udp port 4791 is blocked, the udp port 4891 is used and vice versa.
>> Port 4891 is currently unassigned in the IANA registry. Do you intend
>> to request this? Strongly suggest that this not merge without such a
>> standard.
>>
>> https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt
>>
>>
> The whole idea looks to me like a hack, what will we do once the second
> port blocked too? Will we introduce option to add more ports?
The second port is a backup. When the first port 4791 is blocked, the 
port 4891 will be used. At the same time,
some cleanup work will be done to make udp port 4791 work again. When 
4891 is blocked, 4791 is used again.

It is like failover in bonding.:-)

Zhu Yanjun
>
> Thanks

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhu Yanjun May 13, 2018, 1:12 a.m. UTC | #5
On 2018/5/13 9:05, Yanjun Zhu wrote:
>
>
> On 2018/5/12 22:55, Leon Romanovsky wrote:
>> On Sat, May 12, 2018 at 10:24:03AM -0400, Tom Talpey wrote:
>>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>>> When udp port 4791 is blocked, the udp port 4891 is used and vice 
>>>> versa.
>>> Port 4891 is currently unassigned in the IANA registry. Do you intend
>>> to request this? Strongly suggest that this not merge without such a
>>> standard.
>>>
>>> https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt 
>>>
>>>
>>>
>> The whole idea looks to me like a hack, what will we do once the second
>> port blocked too? Will we introduce option to add more ports?
> The second port is a backup. When the first port 4791 is blocked, the 
> port 4891 will be used. At the same time,
> some cleanup work will be done to make udp port 4791 work again. When 
> 4891 is blocked, 4791 is used again.
This will make soft roce more robust and fault tolerance.

Zhu Yanjun
>
> It is like failover in bonding.:-)
>
> Zhu Yanjun
>>
>> Thanks
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky May 13, 2018, 6:17 a.m. UTC | #6
On Sun, May 13, 2018 at 09:05:45AM +0800, Yanjun Zhu wrote:
>
>
> On 2018/5/12 22:55, Leon Romanovsky wrote:
> > On Sat, May 12, 2018 at 10:24:03AM -0400, Tom Talpey wrote:
> > > On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
> > > > When udp port 4791 is blocked, the udp port 4891 is used and vice versa.
> > > Port 4891 is currently unassigned in the IANA registry. Do you intend
> > > to request this? Strongly suggest that this not merge without such a
> > > standard.
> > >
> > > https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt
> > >
> > >
> > The whole idea looks to me like a hack, what will we do once the second
> > port blocked too? Will we introduce option to add more ports?
> The second port is a backup. When the first port 4791 is blocked, the port
> 4891 will be used. At the same time,
> some cleanup work will be done to make udp port 4791 work again. When 4891
> is blocked, 4791 is used again.
>
> It is like failover in bonding.:-)

Right, so why don't you use bonding for that?

Thanks

>
> Zhu Yanjun
> >
> > Thanks
>
Zhu Yanjun May 13, 2018, 6:22 a.m. UTC | #7
On 2018/5/13 14:17, Leon Romanovsky wrote:
> On Sun, May 13, 2018 at 09:05:45AM +0800, Yanjun Zhu wrote:
>>
>> On 2018/5/12 22:55, Leon Romanovsky wrote:
>>> On Sat, May 12, 2018 at 10:24:03AM -0400, Tom Talpey wrote:
>>>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>>>> When udp port 4791 is blocked, the udp port 4891 is used and vice versa.
>>>> Port 4891 is currently unassigned in the IANA registry. Do you intend
>>>> to request this? Strongly suggest that this not merge without such a
>>>> standard.
>>>>
>>>> https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt
>>>>
>>>>
>>> The whole idea looks to me like a hack, what will we do once the second
>>> port blocked too? Will we introduce option to add more ports?
>> The second port is a backup. When the first port 4791 is blocked, the port
>> 4891 will be used. At the same time,
>> some cleanup work will be done to make udp port 4791 work again. When 4891
>> is blocked, 4791 is used again.
>>
>> It is like failover in bonding.:-)
> Right, so why don't you use bonding for that?
Based on my test results, there is a performance loss with bonding 
compared with this  feature because
the packets will pass bonding driver before directly pass to the real 
physical NIC driver.

So it is necessary to use this feature.

Zhu Yanjun
>
> Thanks
>
>> Zhu Yanjun
>>> Thanks

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky May 13, 2018, 6:36 a.m. UTC | #8
On Sun, May 13, 2018 at 02:22:42PM +0800, Yanjun Zhu wrote:
>
>
> On 2018/5/13 14:17, Leon Romanovsky wrote:
> > On Sun, May 13, 2018 at 09:05:45AM +0800, Yanjun Zhu wrote:
> > >
> > > On 2018/5/12 22:55, Leon Romanovsky wrote:
> > > > On Sat, May 12, 2018 at 10:24:03AM -0400, Tom Talpey wrote:
> > > > > On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
> > > > > > When udp port 4791 is blocked, the udp port 4891 is used and vice versa.
> > > > > Port 4891 is currently unassigned in the IANA registry. Do you intend
> > > > > to request this? Strongly suggest that this not merge without such a
> > > > > standard.
> > > > >
> > > > > https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt
> > > > >
> > > > >
> > > > The whole idea looks to me like a hack, what will we do once the second
> > > > port blocked too? Will we introduce option to add more ports?
> > > The second port is a backup. When the first port 4791 is blocked, the port
> > > 4891 will be used. At the same time,
> > > some cleanup work will be done to make udp port 4791 work again. When 4891
> > > is blocked, 4791 is used again.
> > >
> > > It is like failover in bonding.:-)
> > Right, so why don't you use bonding for that?
> Based on my test results, there is a performance loss with bonding compared
> with this  feature because
> the packets will pass bonding driver before directly pass to the real
> physical NIC driver.

I would like to see results, because RXE performance is far away from
the lane rate and it is hard to imagine that something can hurt it even
more.

Thanks

>
> So it is necessary to use this feature.
>
> Zhu Yanjun
> >
> > Thanks
> >
> > > Zhu Yanjun
> > > > Thanks
>
Zhu Yanjun May 13, 2018, 6:42 a.m. UTC | #9
On 2018/5/13 14:36, Leon Romanovsky wrote:
> On Sun, May 13, 2018 at 02:22:42PM +0800, Yanjun Zhu wrote:
>>
>> On 2018/5/13 14:17, Leon Romanovsky wrote:
>>> On Sun, May 13, 2018 at 09:05:45AM +0800, Yanjun Zhu wrote:
>>>> On 2018/5/12 22:55, Leon Romanovsky wrote:
>>>>> On Sat, May 12, 2018 at 10:24:03AM -0400, Tom Talpey wrote:
>>>>>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>>>>>> When udp port 4791 is blocked, the udp port 4891 is used and vice versa.
>>>>>> Port 4891 is currently unassigned in the IANA registry. Do you intend
>>>>>> to request this? Strongly suggest that this not merge without such a
>>>>>> standard.
>>>>>>
>>>>>> https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt
>>>>>>
>>>>>>
>>>>> The whole idea looks to me like a hack, what will we do once the second
>>>>> port blocked too? Will we introduce option to add more ports?
>>>> The second port is a backup. When the first port 4791 is blocked, the port
>>>> 4891 will be used. At the same time,
>>>> some cleanup work will be done to make udp port 4791 work again. When 4891
>>>> is blocked, 4791 is used again.
>>>>
>>>> It is like failover in bonding.:-)
>>> Right, so why don't you use bonding for that?
>> Based on my test results, there is a performance loss with bonding compared
>> with this  feature because
>> the packets will pass bonding driver before directly pass to the real
>> physical NIC driver.
> I would like to see results, because RXE performance is far away from
> the lane rate and it is hard to imagine that something can hurt it even
> more.

Several weeks, I made tests to compare them. The performance loss is 
about 3%. If you need
the details, I am glad to make tests again.

Zhu Yanjun

>
> Thanks
>
>> So it is necessary to use this feature.
>>
>> Zhu Yanjun
>>> Thanks
>>>
>>>> Zhu Yanjun
>>>>> Thanks

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky May 13, 2018, 7:01 a.m. UTC | #10
On Sun, May 13, 2018 at 02:42:49PM +0800, Yanjun Zhu wrote:
>
>
> On 2018/5/13 14:36, Leon Romanovsky wrote:
> > On Sun, May 13, 2018 at 02:22:42PM +0800, Yanjun Zhu wrote:
> > >
> > > On 2018/5/13 14:17, Leon Romanovsky wrote:
> > > > On Sun, May 13, 2018 at 09:05:45AM +0800, Yanjun Zhu wrote:
> > > > > On 2018/5/12 22:55, Leon Romanovsky wrote:
> > > > > > On Sat, May 12, 2018 at 10:24:03AM -0400, Tom Talpey wrote:
> > > > > > > On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
> > > > > > > > When udp port 4791 is blocked, the udp port 4891 is used and vice versa.
> > > > > > > Port 4891 is currently unassigned in the IANA registry. Do you intend
> > > > > > > to request this? Strongly suggest that this not merge without such a
> > > > > > > standard.
> > > > > > >
> > > > > > > https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt
> > > > > > >
> > > > > > >
> > > > > > The whole idea looks to me like a hack, what will we do once the second
> > > > > > port blocked too? Will we introduce option to add more ports?
> > > > > The second port is a backup. When the first port 4791 is blocked, the port
> > > > > 4891 will be used. At the same time,
> > > > > some cleanup work will be done to make udp port 4791 work again. When 4891
> > > > > is blocked, 4791 is used again.
> > > > >
> > > > > It is like failover in bonding.:-)
> > > > Right, so why don't you use bonding for that?
> > > Based on my test results, there is a performance loss with bonding compared
> > > with this  feature because
> > > the packets will pass bonding driver before directly pass to the real
> > > physical NIC driver.
> > I would like to see results, because RXE performance is far away from
> > the lane rate and it is hard to imagine that something can hurt it even
> > more.
>
> Several weeks, I made tests to compare them. The performance loss is about
> 3%. If you need
> the details, I am glad to make tests again.

IMHO, 3% loss s not worth all this fuss, but let's wait to hear
feedback from other people.

Thanks

>
> Zhu Yanjun
>
> >
> > Thanks
> >
> > > So it is necessary to use this feature.
> > >
> > > Zhu Yanjun
> > > > Thanks
> > > >
> > > > > Zhu Yanjun
> > > > > > Thanks
>
Zhu Yanjun May 13, 2018, 7:09 a.m. UTC | #11
On 2018/5/13 15:01, Leon Romanovsky wrote:
> On Sun, May 13, 2018 at 02:42:49PM +0800, Yanjun Zhu wrote:
>>
>> On 2018/5/13 14:36, Leon Romanovsky wrote:
>>> On Sun, May 13, 2018 at 02:22:42PM +0800, Yanjun Zhu wrote:
>>>> On 2018/5/13 14:17, Leon Romanovsky wrote:
>>>>> On Sun, May 13, 2018 at 09:05:45AM +0800, Yanjun Zhu wrote:
>>>>>> On 2018/5/12 22:55, Leon Romanovsky wrote:
>>>>>>> On Sat, May 12, 2018 at 10:24:03AM -0400, Tom Talpey wrote:
>>>>>>>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>>>>>>>> When udp port 4791 is blocked, the udp port 4891 is used and vice versa.
>>>>>>>> Port 4891 is currently unassigned in the IANA registry. Do you intend
>>>>>>>> to request this? Strongly suggest that this not merge without such a
>>>>>>>> standard.
>>>>>>>>
>>>>>>>> https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt
>>>>>>>>
>>>>>>>>
>>>>>>> The whole idea looks to me like a hack, what will we do once the second
>>>>>>> port blocked too? Will we introduce option to add more ports?
>>>>>> The second port is a backup. When the first port 4791 is blocked, the port
>>>>>> 4891 will be used. At the same time,
>>>>>> some cleanup work will be done to make udp port 4791 work again. When 4891
>>>>>> is blocked, 4791 is used again.
>>>>>>
>>>>>> It is like failover in bonding.:-)
>>>>> Right, so why don't you use bonding for that?
>>>> Based on my test results, there is a performance loss with bonding compared
>>>> with this  feature because
>>>> the packets will pass bonding driver before directly pass to the real
>>>> physical NIC driver.
>>> I would like to see results, because RXE performance is far away from
>>> the lane rate and it is hard to imagine that something can hurt it even
>>> more.
>> Several weeks, I made tests to compare them. The performance loss is about
>> 3%. If you need
>> the details, I am glad to make tests again.
> IMHO, 3% loss s not worth all this fuss,

But if bonding is involved, the complexity of the whole system is 
enhanced. To a production host,
it is not a good thing.

Zhu Yanjun
> but let's wait to hear
> feedback from other people.
>
> Thanks
>
>> Zhu Yanjun
>>
>>> Thanks
>>>
>>>> So it is necessary to use this feature.
>>>>
>>>> Zhu Yanjun
>>>>> Thanks
>>>>>
>>>>>> Zhu Yanjun
>>>>>>> Thanks

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhu Yanjun May 13, 2018, 7:26 a.m. UTC | #12
On 2018/5/13 15:09, Yanjun Zhu wrote:
>
>
> On 2018/5/13 15:01, Leon Romanovsky wrote:
>> On Sun, May 13, 2018 at 02:42:49PM +0800, Yanjun Zhu wrote:
>>>
>>> On 2018/5/13 14:36, Leon Romanovsky wrote:
>>>> On Sun, May 13, 2018 at 02:22:42PM +0800, Yanjun Zhu wrote:
>>>>> On 2018/5/13 14:17, Leon Romanovsky wrote:
>>>>>> On Sun, May 13, 2018 at 09:05:45AM +0800, Yanjun Zhu wrote:
>>>>>>> On 2018/5/12 22:55, Leon Romanovsky wrote:
>>>>>>>> On Sat, May 12, 2018 at 10:24:03AM -0400, Tom Talpey wrote:
>>>>>>>>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>>>>>>>>> When udp port 4791 is blocked, the udp port 4891 is used and 
>>>>>>>>>> vice versa.
>>>>>>>>> Port 4891 is currently unassigned in the IANA registry. Do you 
>>>>>>>>> intend
>>>>>>>>> to request this? Strongly suggest that this not merge without 
>>>>>>>>> such a
>>>>>>>>> standard.
>>>>>>>>>
>>>>>>>>> https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt 
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>> The whole idea looks to me like a hack, what will we do once 
>>>>>>>> the second
>>>>>>>> port blocked too? Will we introduce option to add more ports?
>>>>>>> The second port is a backup. When the first port 4791 is 
>>>>>>> blocked, the port
>>>>>>> 4891 will be used. At the same time,
>>>>>>> some cleanup work will be done to make udp port 4791 work again. 
>>>>>>> When 4891
>>>>>>> is blocked, 4791 is used again.
>>>>>>>
>>>>>>> It is like failover in bonding.:-)
>>>>>> Right, so why don't you use bonding for that?
>>>>> Based on my test results, there is a performance loss with bonding 
>>>>> compared
>>>>> with this  feature because
>>>>> the packets will pass bonding driver before directly pass to the real
>>>>> physical NIC driver.
>>>> I would like to see results, because RXE performance is far away from
>>>> the lane rate and it is hard to imagine that something can hurt it 
>>>> even
>>>> more.
>>> Several weeks, I made tests to compare them. The performance loss is 
>>> about
>>> 3%. If you need
>>> the details, I am glad to make tests again.
>> IMHO, 3% loss s not worth all this fuss,
>
> But if bonding is involved, the complexity of the whole system is 
> enhanced. To a production host,
> it is not a good thing.

And with bonding, more NIC devices are needed when soft roce is used in 
the physical host. But with this feature,
less NIC devices are needed.:-P

Zhu Yanjun

>
> Zhu Yanjun
>> but let's wait to hear
>> feedback from other people.
>>
>> Thanks
>>
>>> Zhu Yanjun
>>>
>>>> Thanks
>>>>
>>>>> So it is necessary to use this feature.
>>>>>
>>>>> Zhu Yanjun
>>>>>> Thanks
>>>>>>
>>>>>>> Zhu Yanjun
>>>>>>>> Thanks
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tom Talpey May 13, 2018, 11:49 a.m. UTC | #13
On 5/12/2018 9:00 PM, Yanjun Zhu wrote:
> 
> 
> On 2018/5/12 22:24, Tom Talpey wrote:
>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>> When udp port 4791 is blocked, the udp port 4891 is used and vice versa.
>>
>> Port 4891 is currently unassigned in the IANA registry. Do you intend
>> to request this? 
> Yes. If this patch is merged, I will request it.

That is improper - the standards assignment must come first. In my
opinion the firewall reasoning will be rejected by the IANA, and
there's no guarantee what number will be assigned in any case. The
process is defined here: https://tools.ietf.org/html/rfc6335

Do the hard stuff first; develop a solid justification why this is
a good and proper approach, before deploying it on the IP Internet
by making it part of Linux.

Tom.

> 
> Zhu Yanjun
>> Strongly suggest that this not merge without such a
>> standard.
>>
>> https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt 
>>
>>
>>
>>>
>>> CC: Srinivas Eeda <srinivas.eeda@oracle.com>
>>> CC: Junxiao Bi <junxiao.bi@oracle.com>
>>> Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
>>> ---
>>>   drivers/infiniband/sw/rxe/rxe_net.c | 13 +++++++++++--
>>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c 
>>> b/drivers/infiniband/sw/rxe/rxe_net.c
>>> index 29f6937..d61f2c5 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
>>> @@ -276,6 +276,16 @@ static inline void prepare_udp_ports_spin(__be16 
>>> dport, __be16 sport)
>>>       src_port = sport;
>>>       spin_unlock(&port_lock);
>>>   }
>>> +
>>> +static inline void exchange_udp_port(void)
>>> +{
>>> +    if (dst_port == htons(ROCE_V2_UDP_DPORT))
>>> +        prepare_udp_ports_spin(htons(ROCE_V2_UDP_DUAL_DPORT),
>>> +                       htons(RXE_ROCE_V2_DUAL_SPORT));
>>> +    else
>>> +        prepare_udp_ports_spin(htons(ROCE_V2_UDP_DPORT),
>>> +                       htons(RXE_ROCE_V2_SPORT));
>>> +}
>>>   #endif
>>>     static int rxe_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
>>> @@ -549,8 +559,7 @@ int rxe_send(struct rxe_pkt_info *pkt, struct 
>>> sk_buff *skb)
>>>     #ifdef CONFIG_RDMA_RXE_DUAL_PORT_MODULE
>>>       if (unlikely(err != NET_XMIT_SUCCESS))
>>> -        prepare_udp_ports_spin(htons(ROCE_V2_UDP_DUAL_DPORT),
>>> -                       htons(RXE_ROCE_V2_DUAL_SPORT));
>>> +        exchange_udp_port();
>>>   #endif
>>>         if (unlikely(net_xmit_eval(err))) {
>>>
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhu Yanjun May 14, 2018, 1:42 a.m. UTC | #14
On 2018/5/13 19:49, Tom Talpey wrote:
> On 5/12/2018 9:00 PM, Yanjun Zhu wrote:
>>
>>
>> On 2018/5/12 22:24, Tom Talpey wrote:
>>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>>> When udp port 4791 is blocked, the udp port 4891 is used and vice 
>>>> versa.
>>>
>>> Port 4891 is currently unassigned in the IANA registry. Do you intend
>>> to request this? 
>> Yes. If this patch is merged, I will request it.
>
> That is improper - the standards assignment must come first. In my
> opinion the firewall reasoning will be rejected by the IANA, and
> there's no guarantee what number will be assigned in any case. The
> process is defined here: https://tools.ietf.org/html/rfc6335
>
> Do the hard stuff first; develop a solid justification why this is
> a good and proper approach, before deploying it on the IP Internet
> by making it part of Linux.
Thanks a lot for your suggestions. I will request the udp port now.

Zhu Yanjun
>
> Tom.
>
>>
>> Zhu Yanjun
>>> Strongly suggest that this not merge without such a
>>> standard.
>>>
>>> https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt 
>>>
>>>
>>>
>>>>
>>>> CC: Srinivas Eeda <srinivas.eeda@oracle.com>
>>>> CC: Junxiao Bi <junxiao.bi@oracle.com>
>>>> Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
>>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
>>>> ---
>>>>   drivers/infiniband/sw/rxe/rxe_net.c | 13 +++++++++++--
>>>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c 
>>>> b/drivers/infiniband/sw/rxe/rxe_net.c
>>>> index 29f6937..d61f2c5 100644
>>>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
>>>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
>>>> @@ -276,6 +276,16 @@ static inline void 
>>>> prepare_udp_ports_spin(__be16 dport, __be16 sport)
>>>>       src_port = sport;
>>>>       spin_unlock(&port_lock);
>>>>   }
>>>> +
>>>> +static inline void exchange_udp_port(void)
>>>> +{
>>>> +    if (dst_port == htons(ROCE_V2_UDP_DPORT))
>>>> + prepare_udp_ports_spin(htons(ROCE_V2_UDP_DUAL_DPORT),
>>>> +                       htons(RXE_ROCE_V2_DUAL_SPORT));
>>>> +    else
>>>> +        prepare_udp_ports_spin(htons(ROCE_V2_UDP_DPORT),
>>>> +                       htons(RXE_ROCE_V2_SPORT));
>>>> +}
>>>>   #endif
>>>>     static int rxe_udp_encap_recv(struct sock *sk, struct sk_buff 
>>>> *skb)
>>>> @@ -549,8 +559,7 @@ int rxe_send(struct rxe_pkt_info *pkt, struct 
>>>> sk_buff *skb)
>>>>     #ifdef CONFIG_RDMA_RXE_DUAL_PORT_MODULE
>>>>       if (unlikely(err != NET_XMIT_SUCCESS))
>>>> - prepare_udp_ports_spin(htons(ROCE_V2_UDP_DUAL_DPORT),
>>>> -                       htons(RXE_ROCE_V2_DUAL_SPORT));
>>>> +        exchange_udp_port();
>>>>   #endif
>>>>         if (unlikely(net_xmit_eval(err))) {
>>>>
>>>
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe May 14, 2018, 7:41 p.m. UTC | #15
On Mon, May 14, 2018 at 09:42:31AM +0800, Yanjun Zhu wrote:
> 
> 
> On 2018/5/13 19:49, Tom Talpey wrote:
> >On 5/12/2018 9:00 PM, Yanjun Zhu wrote:
> >>
> >>
> >>On 2018/5/12 22:24, Tom Talpey wrote:
> >>>On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
> >>>>When udp port 4791 is blocked, the udp port 4891 is used and vice
> >>>>versa.
> >>>
> >>>Port 4891 is currently unassigned in the IANA registry. Do you intend
> >>>to request this?
> >>Yes. If this patch is merged, I will request it.
> >
> >That is improper - the standards assignment must come first. In my
> >opinion the firewall reasoning will be rejected by the IANA, and
> >there's no guarantee what number will be assigned in any case. The
> >process is defined here: https://tools.ietf.org/html/rfc6335
> >
> >Do the hard stuff first; develop a solid justification why this is
> >a good and proper approach, before deploying it on the IP Internet
> >by making it part of Linux.
> Thanks a lot for your suggestions. I will request the udp port now.

You need to start with IBTA as they control the standard and IETF
probably won't give you a ROCE related port without their approval
either.

Nor are we likely to add non-standard ROCE behavior to rxe.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhu Yanjun May 14, 2018, 11:23 p.m. UTC | #16
On 2018/5/15 3:41, Jason Gunthorpe wrote:
> On Mon, May 14, 2018 at 09:42:31AM +0800, Yanjun Zhu wrote:
>>
>> On 2018/5/13 19:49, Tom Talpey wrote:
>>> On 5/12/2018 9:00 PM, Yanjun Zhu wrote:
>>>>
>>>> On 2018/5/12 22:24, Tom Talpey wrote:
>>>>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>>>>> When udp port 4791 is blocked, the udp port 4891 is used and vice
>>>>>> versa.
>>>>> Port 4891 is currently unassigned in the IANA registry. Do you intend
>>>>> to request this?
>>>> Yes. If this patch is merged, I will request it.
>>> That is improper - the standards assignment must come first. In my
>>> opinion the firewall reasoning will be rejected by the IANA, and
>>> there's no guarantee what number will be assigned in any case. The
>>> process is defined here: https://tools.ietf.org/html/rfc6335
>>>
>>> Do the hard stuff first; develop a solid justification why this is
>>> a good and proper approach, before deploying it on the IP Internet
>>> by making it part of Linux.
>> Thanks a lot for your suggestions. I will request the udp port now.
> You need to start with IBTA as they control the standard and IETF
> probably won't give you a ROCE related port without their approval
> either.
Thanks a lot for your suggestions. But how to start with IBTA? What 
should I do to start with IBTA?
Who should I contact?

Thanks a lot.
Zhu Yanjun
>
> Nor are we likely to add non-standard ROCE behavior to rxe.
>
> Jason
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhu Yanjun May 15, 2018, 12:24 p.m. UTC | #17
On 2018/5/15 3:41, Jason Gunthorpe wrote:
> On Mon, May 14, 2018 at 09:42:31AM +0800, Yanjun Zhu wrote:
>>
>> On 2018/5/13 19:49, Tom Talpey wrote:
>>> On 5/12/2018 9:00 PM, Yanjun Zhu wrote:
>>>>
>>>> On 2018/5/12 22:24, Tom Talpey wrote:
>>>>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>>>>> When udp port 4791 is blocked, the udp port 4891 is used and vice
>>>>>> versa.
>>>>> Port 4891 is currently unassigned in the IANA registry. Do you intend
>>>>> to request this?
>>>> Yes. If this patch is merged, I will request it.
>>> That is improper - the standards assignment must come first. In my
>>> opinion the firewall reasoning will be rejected by the IANA, and
>>> there's no guarantee what number will be assigned in any case. The
>>> process is defined here: https://tools.ietf.org/html/rfc6335
>>>
>>> Do the hard stuff first; develop a solid justification why this is
>>> a good and proper approach, before deploying it on the IP Internet
>>> by making it part of Linux.
>> Thanks a lot for your suggestions. I will request the udp port now.
Hi, Jason

I checked the Annex 17: RoCEv2. From this spec, the udp is specified to 
carry rdma.
This feature is based on the above. And it is transparent to RDMA. That 
is, RDMA will
not detect udp port exchange. And this feature is helpful to RDMA.

So I do not know why IBTA do not want to give us a ROCE related port. I 
will request the port
with IETF.

If I can get the port, will you merge these patches into mainline?

Zhu Yanjun
> You need to start with IBTA as they control the standard and IETF
> probably won't give you a ROCE related port without their approval
> either.
>
> Nor are we likely to add non-standard ROCE behavior to rxe.
>
> Jason
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe May 15, 2018, 2:45 p.m. UTC | #18
On Tue, May 15, 2018 at 08:24:37PM +0800, Yanjun Zhu wrote:
> 
> 
> On 2018/5/15 3:41, Jason Gunthorpe wrote:
> >On Mon, May 14, 2018 at 09:42:31AM +0800, Yanjun Zhu wrote:
> >>
> >>On 2018/5/13 19:49, Tom Talpey wrote:
> >>>On 5/12/2018 9:00 PM, Yanjun Zhu wrote:
> >>>>
> >>>>On 2018/5/12 22:24, Tom Talpey wrote:
> >>>>>On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
> >>>>>>When udp port 4791 is blocked, the udp port 4891 is used and vice
> >>>>>>versa.
> >>>>>Port 4891 is currently unassigned in the IANA registry. Do you intend
> >>>>>to request this?
> >>>>Yes. If this patch is merged, I will request it.
> >>>That is improper - the standards assignment must come first. In my
> >>>opinion the firewall reasoning will be rejected by the IANA, and
> >>>there's no guarantee what number will be assigned in any case. The
> >>>process is defined here: https://tools.ietf.org/html/rfc6335
> >>>
> >>>Do the hard stuff first; develop a solid justification why this is
> >>>a good and proper approach, before deploying it on the IP Internet
> >>>by making it part of Linux.
> >>Thanks a lot for your suggestions. I will request the udp port now.
> Hi, Jason
> 
> I checked the Annex 17: RoCEv2. From this spec, the udp is specified to
> carry rdma.
> This feature is based on the above. And it is transparent to RDMA. That is,
> RDMA will
> not detect udp port exchange. And this feature is helpful to RDMA.

IBTA controls the port numbers that rocev2 uses, and how it should
intereact if there are suddently two port numbers.

> So I do not know why IBTA do not want to give us a ROCE related port. I will
> request the port
> with IETF.
> 
> If I can get the port, will you merge these patches into mainline?

Not without an IBTA spec explaining the behavior so this can be
interoperable.

Talk to your company's IBTA representatives for guidance on the
process.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Parav Pandit May 15, 2018, 3:42 p.m. UTC | #19
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Jason Gunthorpe
> Sent: Tuesday, May 15, 2018 9:46 AM
> To: Yanjun Zhu <yanjun.zhu@oracle.com>
> Cc: Tom Talpey <tom@talpey.com>; Moni Shoua <monis@mellanox.com>;
> dledford@redhat.com; linux-rdma@vger.kernel.org
> Subject: Re: [PATCH 4/4] IB/rxe: exchange the 2 udp ports
> 
> On Tue, May 15, 2018 at 08:24:37PM +0800, Yanjun Zhu wrote:
> >
> >
> > On 2018/5/15 3:41, Jason Gunthorpe wrote:
> > >On Mon, May 14, 2018 at 09:42:31AM +0800, Yanjun Zhu wrote:
> > >>
> > >>On 2018/5/13 19:49, Tom Talpey wrote:
> > >>>On 5/12/2018 9:00 PM, Yanjun Zhu wrote:
> > >>>>
> > >>>>On 2018/5/12 22:24, Tom Talpey wrote:
> > >>>>>On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
> > >>>>>>When udp port 4791 is blocked, the udp port 4891 is used and
> > >>>>>>vice versa.
> > >>>>>Port 4891 is currently unassigned in the IANA registry. Do you
> > >>>>>intend to request this?
> > >>>>Yes. If this patch is merged, I will request it.
> > >>>That is improper - the standards assignment must come first. In my
> > >>>opinion the firewall reasoning will be rejected by the IANA, and
> > >>>there's no guarantee what number will be assigned in any case. The
> > >>>process is defined here: https://tools.ietf.org/html/rfc6335
> > >>>
> > >>>Do the hard stuff first; develop a solid justification why this is
> > >>>a good and proper approach, before deploying it on the IP Internet
> > >>>by making it part of Linux.
> > >>Thanks a lot for your suggestions. I will request the udp port now.
> > Hi, Jason
> >
> > I checked the Annex 17: RoCEv2. From this spec, the udp is specified
> > to carry rdma.
> > This feature is based on the above. And it is transparent to RDMA.
> > That is, RDMA will not detect udp port exchange. And this feature is
> > helpful to RDMA.
> 
> IBTA controls the port numbers that rocev2 uses, and how it should intereact if
> there are suddently two port numbers.
> 
> > So I do not know why IBTA do not want to give us a ROCE related port.
> > I will request the port with IETF.
> >
> > If I can get the port, will you merge these patches into mainline?
> 
I don't think this is right direction.
I don't see there is any problem here that requires IBTA or IETF's attention.
In fact this method is creating more security issues and requires mores administrative hand holding where now administrator needs to block multiple UDP ports to disable RoCE.
Leon already explained this.

What is the inspiration for administrator to block port 4791 and not port 4891, where the service is offered on port 4791.
Why vxlan UDP encapsulation doesn't face this problem, which likely/might have wider user base.
You intent to create a software extension which bypasses administrative policy; - not a good idea.

Again rxe will not be able have control of the udp ports that it intend to use, even if it is coded this way currently.
It will be ib_core. So the patches in current form won't make progress until the ib_core rework is finished.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhu Yanjun May 16, 2018, 3:27 a.m. UTC | #20
On 2018/5/15 23:42, Parav Pandit wrote:
>
>> -----Original Message-----
>> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
>> owner@vger.kernel.org] On Behalf Of Jason Gunthorpe
>> Sent: Tuesday, May 15, 2018 9:46 AM
>> To: Yanjun Zhu <yanjun.zhu@oracle.com>
>> Cc: Tom Talpey <tom@talpey.com>; Moni Shoua <monis@mellanox.com>;
>> dledford@redhat.com; linux-rdma@vger.kernel.org
>> Subject: Re: [PATCH 4/4] IB/rxe: exchange the 2 udp ports
>>
>> On Tue, May 15, 2018 at 08:24:37PM +0800, Yanjun Zhu wrote:
>>>
>>> On 2018/5/15 3:41, Jason Gunthorpe wrote:
>>>> On Mon, May 14, 2018 at 09:42:31AM +0800, Yanjun Zhu wrote:
>>>>> On 2018/5/13 19:49, Tom Talpey wrote:
>>>>>> On 5/12/2018 9:00 PM, Yanjun Zhu wrote:
>>>>>>> On 2018/5/12 22:24, Tom Talpey wrote:
>>>>>>>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:
>>>>>>>>> When udp port 4791 is blocked, the udp port 4891 is used and
>>>>>>>>> vice versa.
>>>>>>>> Port 4891 is currently unassigned in the IANA registry. Do you
>>>>>>>> intend to request this?
>>>>>>> Yes. If this patch is merged, I will request it.
>>>>>> That is improper - the standards assignment must come first. In my
>>>>>> opinion the firewall reasoning will be rejected by the IANA, and
>>>>>> there's no guarantee what number will be assigned in any case. The
>>>>>> process is defined here: https://tools.ietf.org/html/rfc6335
>>>>>>
>>>>>> Do the hard stuff first; develop a solid justification why this is
>>>>>> a good and proper approach, before deploying it on the IP Internet
>>>>>> by making it part of Linux.
>>>>> Thanks a lot for your suggestions. I will request the udp port now.
>>> Hi, Jason
>>>
>>> I checked the Annex 17: RoCEv2. From this spec, the udp is specified
>>> to carry rdma.
>>> This feature is based on the above. And it is transparent to RDMA.
>>> That is, RDMA will not detect udp port exchange. And this feature is
>>> helpful to RDMA.
>> IBTA controls the port numbers that rocev2 uses, and how it should intereact if
>> there are suddently two port numbers.
>>
>>> So I do not know why IBTA do not want to give us a ROCE related port.
>>> I will request the port with IETF.
>>>
>>> If I can get the port, will you merge these patches into mainline?
> I don't think this is right direction.
> I don't see there is any problem here that requires IBTA or IETF's attention.
> In fact this method is creating more security issues and requires mores administrative hand holding where now administrator needs to block multiple UDP ports to disable RoCE.
Why is this method creating more security issues? Does another udp port 
in this method cause more security issues?

> Leon already explained this.
>
> What is the inspiration for administrator to block port 4791 and not port 4891, where the service is offered on port 4791
> Why vxlan UDP encapsulation doesn't face this problem, which likely/might have wider user base.
> You intent to create a software extension which bypasses administrative policy; - not a good idea.
>
> Again rxe will not be able have control of the udp ports that it intend to use, even if it is coded this way currently.
Sorry. I do not think it is a good idea that ib_core controls upd ports.
According to the layering principle, network should control udp port.
RDMA controls its affairs.
Network and RDMA should split.

Zhu Yanjun
> It will be ib_core. So the patches in current form won't make progress until the ib_core rework is finished.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Parav Pandit May 16, 2018, 3:38 a.m. UTC | #21
> -----Original Message-----

> From: Yanjun Zhu [mailto:yanjun.zhu@oracle.com]

> Sent: Tuesday, May 15, 2018 10:28 PM

> To: Parav Pandit <parav@mellanox.com>; Jason Gunthorpe <jgg@ziepe.ca>

> Cc: Tom Talpey <tom@talpey.com>; Moni Shoua <monis@mellanox.com>;

> dledford@redhat.com; linux-rdma@vger.kernel.org

> Subject: Re: [PATCH 4/4] IB/rxe: exchange the 2 udp ports

> 

> 

> 

> On 2018/5/15 23:42, Parav Pandit wrote:

> >

> >> -----Original Message-----

> >> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-

> >> owner@vger.kernel.org] On Behalf Of Jason Gunthorpe

> >> Sent: Tuesday, May 15, 2018 9:46 AM

> >> To: Yanjun Zhu <yanjun.zhu@oracle.com>

> >> Cc: Tom Talpey <tom@talpey.com>; Moni Shoua <monis@mellanox.com>;

> >> dledford@redhat.com; linux-rdma@vger.kernel.org

> >> Subject: Re: [PATCH 4/4] IB/rxe: exchange the 2 udp ports

> >>

> >> On Tue, May 15, 2018 at 08:24:37PM +0800, Yanjun Zhu wrote:

> >>>

> >>> On 2018/5/15 3:41, Jason Gunthorpe wrote:

> >>>> On Mon, May 14, 2018 at 09:42:31AM +0800, Yanjun Zhu wrote:

> >>>>> On 2018/5/13 19:49, Tom Talpey wrote:

> >>>>>> On 5/12/2018 9:00 PM, Yanjun Zhu wrote:

> >>>>>>> On 2018/5/12 22:24, Tom Talpey wrote:

> >>>>>>>> On 5/12/2018 9:54 AM, Zhu Yanjun wrote:

> >>>>>>>>> When udp port 4791 is blocked, the udp port 4891 is used and

> >>>>>>>>> vice versa.

> >>>>>>>> Port 4891 is currently unassigned in the IANA registry. Do you

> >>>>>>>> intend to request this?

> >>>>>>> Yes. If this patch is merged, I will request it.

> >>>>>> That is improper - the standards assignment must come first. In

> >>>>>> my opinion the firewall reasoning will be rejected by the IANA,

> >>>>>> and there's no guarantee what number will be assigned in any

> >>>>>> case. The process is defined here:

> >>>>>> https://tools.ietf.org/html/rfc6335

> >>>>>>

> >>>>>> Do the hard stuff first; develop a solid justification why this

> >>>>>> is a good and proper approach, before deploying it on the IP

> >>>>>> Internet by making it part of Linux.

> >>>>> Thanks a lot for your suggestions. I will request the udp port now.

> >>> Hi, Jason

> >>>

> >>> I checked the Annex 17: RoCEv2. From this spec, the udp is specified

> >>> to carry rdma.

> >>> This feature is based on the above. And it is transparent to RDMA.

> >>> That is, RDMA will not detect udp port exchange. And this feature is

> >>> helpful to RDMA.

> >> IBTA controls the port numbers that rocev2 uses, and how it should

> >> intereact if there are suddently two port numbers.

> >>

> >>> So I do not know why IBTA do not want to give us a ROCE related port.

> >>> I will request the port with IETF.

> >>>

> >>> If I can get the port, will you merge these patches into mainline?

> > I don't think this is right direction.

> > I don't see there is any problem here that requires IBTA or IETF's attention.

> > In fact this method is creating more security issues and requires mores

> administrative hand holding where now administrator needs to block multiple

> UDP ports to disable RoCE.

> Why is this method creating more security issues? Does another udp port in this

> method cause more security issues?

> 

> > Leon already explained this.

> >

> > What is the inspiration for administrator to block port 4791 and not

> > port 4891, where the service is offered on port 4791 Why vxlan UDP

> encapsulation doesn't face this problem, which likely/might have wider user

> base.

> > You intent to create a software extension which bypasses administrative

> policy; - not a good idea.

> >

> > Again rxe will not be able have control of the udp ports that it intend to use,

> even if it is coded this way currently.

> Sorry. I do not think it is a good idea that ib_core controls upd ports.

> According to the layering principle, network should control udp port.

> RDMA controls its affairs.

> Network and RDMA should split.

RoCEv2 is networking over UDP. Provider driver (rxe) cannot own the UDP port.
It is in control of ib core to provide right socket to use; it doesn’t matter whether it is hw/sw provider.

> 

> Zhu Yanjun

> > It will be ib_core. So the patches in current form won't make progress until the

> ib_core rework is finished.

> >
diff mbox

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 29f6937..d61f2c5 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -276,6 +276,16 @@  static inline void prepare_udp_ports_spin(__be16 dport, __be16 sport)
 	src_port = sport;
 	spin_unlock(&port_lock);
 }
+
+static inline void exchange_udp_port(void)
+{
+	if (dst_port == htons(ROCE_V2_UDP_DPORT))
+		prepare_udp_ports_spin(htons(ROCE_V2_UDP_DUAL_DPORT),
+				       htons(RXE_ROCE_V2_DUAL_SPORT));
+	else
+		prepare_udp_ports_spin(htons(ROCE_V2_UDP_DPORT),
+				       htons(RXE_ROCE_V2_SPORT));
+}
 #endif
 
 static int rxe_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
@@ -549,8 +559,7 @@  int rxe_send(struct rxe_pkt_info *pkt, struct sk_buff *skb)
 
 #ifdef CONFIG_RDMA_RXE_DUAL_PORT_MODULE
 	if (unlikely(err != NET_XMIT_SUCCESS))
-		prepare_udp_ports_spin(htons(ROCE_V2_UDP_DUAL_DPORT),
-				       htons(RXE_ROCE_V2_DUAL_SPORT));
+		exchange_udp_port();
 #endif
 
 	if (unlikely(net_xmit_eval(err))) {