* [PATCH] mptcp: create listening socket only if signal flag is set on
@ 2021-04-22 10:30 Jianguo Wu
2021-04-23 4:33 ` Geliang Tang
0 siblings, 1 reply; 14+ messages in thread
From: Jianguo Wu @ 2021-04-22 10:30 UTC (permalink / raw)
To: mptcp; +Cc: geliangtang
From: Jianguo Wu <wujianguo@chinatelecom.cn>
PM announces address to remote host when signal flag is set,
so create listening socket only if signal flag is set.
Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
---
net/mptcp/pm_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index becc32b..dbd6910 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1025,7 +1025,7 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
}
*entry = addr;
- if (entry->addr.port) {
+ if (entry->addr.port && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
if (ret) {
GENL_SET_ERR_MSG(info, "create listen socket error");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-22 10:30 [PATCH] mptcp: create listening socket only if signal flag is set on Jianguo Wu
@ 2021-04-23 4:33 ` Geliang Tang
2021-04-23 6:38 ` Jianguo Wu
0 siblings, 1 reply; 14+ messages in thread
From: Geliang Tang @ 2021-04-23 4:33 UTC (permalink / raw)
To: Jianguo Wu; +Cc: mptcp
Hi Jianguo,
Thanks for your patch.
Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
>
> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>
> PM announces address to remote host when signal flag is set,
> so create listening socket only if signal flag is set.
This check has done in the user-space, here are these lines in pm_nl_ctl.c:
274 } else if (!strcmp(argv[arg], "port")) {
275 u_int16_t port;
276
277 if (++arg >= argc)
278 error(1, 0, " missing port value");
279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
280 error(1, 0, " flags must be signal
when using port");
-Geliang
>
> Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
> ---
> net/mptcp/pm_netlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index becc32b..dbd6910 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1025,7 +1025,7 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
> }
>
> *entry = addr;
> - if (entry->addr.port) {
> + if (entry->addr.port && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
> if (ret) {
> GENL_SET_ERR_MSG(info, "create listen socket error");
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 4:33 ` Geliang Tang
@ 2021-04-23 6:38 ` Jianguo Wu
2021-04-23 7:17 ` Geliang Tang
0 siblings, 1 reply; 14+ messages in thread
From: Jianguo Wu @ 2021-04-23 6:38 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
On 2021/4/23 12:33, Geliang Tang wrote:
> Hi Jianguo,
>
> Thanks for your patch.
>
> Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
>>
>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>>
>> PM announces address to remote host when signal flag is set,
>> so create listening socket only if signal flag is set.
>
> This check has done in the user-space, here are these lines in pm_nl_ctl.c:
>
> 274 } else if (!strcmp(argv[arg], "port")) {
> 275 u_int16_t port;
> 276
> 277 if (++arg >= argc)
> 278 error(1, 0, " missing port value");
> 279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> 280 error(1, 0, " flags must be signal
> when using port");
>
Hi Geliang, Thanks for point out this, I was use 'ip mptcp' to add address, It doesn't seem to have done that check.
Is it worth to do check in kernel to make sure there is a signal flag before create listening socket?
> -Geliang
>
>>
>> Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
>> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
>> ---
>> net/mptcp/pm_netlink.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>> index becc32b..dbd6910 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c
>> @@ -1025,7 +1025,7 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
>> }
>>
>> *entry = addr;
>> - if (entry->addr.port) {
>> + if (entry->addr.port && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
>> ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
>> if (ret) {
>> GENL_SET_ERR_MSG(info, "create listen socket error");
>> --
>> 1.8.3.1
>>
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 6:38 ` Jianguo Wu
@ 2021-04-23 7:17 ` Geliang Tang
2021-04-23 7:27 ` Geliang Tang
2021-04-23 7:28 ` Jianguo Wu
0 siblings, 2 replies; 14+ messages in thread
From: Geliang Tang @ 2021-04-23 7:17 UTC (permalink / raw)
To: Jianguo Wu; +Cc: mptcp
Hi Jianguo,
Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午2:39写道:
>
>
>
> On 2021/4/23 12:33, Geliang Tang wrote:
> > Hi Jianguo,
> >
> > Thanks for your patch.
> >
> > Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
> >>
> >> From: Jianguo Wu <wujianguo@chinatelecom.cn>
> >>
> >> PM announces address to remote host when signal flag is set,
> >> so create listening socket only if signal flag is set.
> >
> > This check has done in the user-space, here are these lines in pm_nl_ctl.c:
> >
> > 274 } else if (!strcmp(argv[arg], "port")) {
> > 275 u_int16_t port;
> > 276
> > 277 if (++arg >= argc)
> > 278 error(1, 0, " missing port value");
> > 279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> > 280 error(1, 0, " flags must be signal
> > when using port");
> >
>
> Hi Geliang, Thanks for point out this, I was use 'ip mptcp' to add address, It doesn't seem to have done that check.
> Is it worth to do check in kernel to make sure there is a signal flag before create listening socket?
I think it's better to check this in the user-space, you can write a patch
to iproute2 to add this check in 'ip mptcp'.
Thanks.
-Geliang
>
> > -Geliang
> >
> >>
> >> Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
> >> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
> >> ---
> >> net/mptcp/pm_netlink.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> >> index becc32b..dbd6910 100644
> >> --- a/net/mptcp/pm_netlink.c
> >> +++ b/net/mptcp/pm_netlink.c
> >> @@ -1025,7 +1025,7 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
> >> }
> >>
> >> *entry = addr;
> >> - if (entry->addr.port) {
> >> + if (entry->addr.port && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> >> ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
> >> if (ret) {
> >> GENL_SET_ERR_MSG(info, "create listen socket error");
> >> --
> >> 1.8.3.1
> >>
> >>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 7:17 ` Geliang Tang
@ 2021-04-23 7:27 ` Geliang Tang
2021-04-23 7:41 ` Jianguo Wu
2021-04-23 7:28 ` Jianguo Wu
1 sibling, 1 reply; 14+ messages in thread
From: Geliang Tang @ 2021-04-23 7:27 UTC (permalink / raw)
To: Jianguo Wu; +Cc: mptcp
Geliang Tang <geliangtang@gmail.com> 于2021年4月23日周五 下午3:17写道:
>
> Hi Jianguo,
>
> Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午2:39写道:
> >
> >
> >
> > On 2021/4/23 12:33, Geliang Tang wrote:
> > > Hi Jianguo,
> > >
> > > Thanks for your patch.
> > >
> > > Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
> > >>
> > >> From: Jianguo Wu <wujianguo@chinatelecom.cn>
> > >>
> > >> PM announces address to remote host when signal flag is set,
> > >> so create listening socket only if signal flag is set.
> > >
> > > This check has done in the user-space, here are these lines in pm_nl_ctl.c:
> > >
> > > 274 } else if (!strcmp(argv[arg], "port")) {
> > > 275 u_int16_t port;
> > > 276
> > > 277 if (++arg >= argc)
> > > 278 error(1, 0, " missing port value");
> > > 279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> > > 280 error(1, 0, " flags must be signal
> > > when using port");
> > >
> >
> > Hi Geliang, Thanks for point out this, I was use 'ip mptcp' to add address, It doesn't seem to have done that check.
> > Is it worth to do check in kernel to make sure there is a signal flag before create listening socket?
>
> I think it's better to check this in the user-space, you can write a patch
> to iproute2 to add this check in 'ip mptcp'.
Or add this check in mptcp_pm_parse_addr, something like:
$ git diff net/mptcp/pm_netlink.c
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 6ba040897738..9cd4a4180a2a 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -971,8 +971,14 @@ static int mptcp_pm_parse_addr(struct nlattr
*attr, struct genl_info *info,
if (tb[MPTCP_PM_ADDR_ATTR_FLAGS])
entry->flags = nla_get_u32(tb[MPTCP_PM_ADDR_ATTR_FLAGS]);
- if (tb[MPTCP_PM_ADDR_ATTR_PORT])
+ if (tb[MPTCP_PM_ADDR_ATTR_PORT]) {
+ if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
+ NL_SET_ERR_MSG_ATTR("");
+ return -EINVAL;
+ }
+
entry->addr.port =
htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]));
+ }
return 0;
}
WDYT?
-Geliang
>
> Thanks.
>
> -Geliang
>
> >
> > > -Geliang
> > >
> > >>
> > >> Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
> > >> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
> > >> ---
> > >> net/mptcp/pm_netlink.c | 2 +-
> > >> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > >> index becc32b..dbd6910 100644
> > >> --- a/net/mptcp/pm_netlink.c
> > >> +++ b/net/mptcp/pm_netlink.c
> > >> @@ -1025,7 +1025,7 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
> > >> }
> > >>
> > >> *entry = addr;
> > >> - if (entry->addr.port) {
> > >> + if (entry->addr.port && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> > >> ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
> > >> if (ret) {
> > >> GENL_SET_ERR_MSG(info, "create listen socket error");
> > >> --
> > >> 1.8.3.1
> > >>
> > >>
> >
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 7:17 ` Geliang Tang
2021-04-23 7:27 ` Geliang Tang
@ 2021-04-23 7:28 ` Jianguo Wu
1 sibling, 0 replies; 14+ messages in thread
From: Jianguo Wu @ 2021-04-23 7:28 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
On 2021/4/23 15:17, Geliang Tang wrote:
> Hi Jianguo,
>
> Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午2:39写道:
>>
>>
>>
>> On 2021/4/23 12:33, Geliang Tang wrote:
>>> Hi Jianguo,
>>>
>>> Thanks for your patch.
>>>
>>> Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
>>>>
>>>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>>>>
>>>> PM announces address to remote host when signal flag is set,
>>>> so create listening socket only if signal flag is set.
>>>
>>> This check has done in the user-space, here are these lines in pm_nl_ctl.c:
>>>
>>> 274 } else if (!strcmp(argv[arg], "port")) {
>>> 275 u_int16_t port;
>>> 276
>>> 277 if (++arg >= argc)
>>> 278 error(1, 0, " missing port value");
>>> 279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
>>> 280 error(1, 0, " flags must be signal
>>> when using port");
>>>
>>
>> Hi Geliang, Thanks for point out this, I was use 'ip mptcp' to add address, It doesn't seem to have done that check.
>> Is it worth to do check in kernel to make sure there is a signal flag before create listening socket?
>
> I think it's better to check this in the user-space, you can write a patch
> to iproute2 to add this check in 'ip mptcp'.
>
OK, thanks!
> Thanks.
>
> -Geliang
>
>>
>>> -Geliang
>>>
>>>>
>>>> Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
>>>> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
>>>> ---
>>>> net/mptcp/pm_netlink.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>>>> index becc32b..dbd6910 100644
>>>> --- a/net/mptcp/pm_netlink.c
>>>> +++ b/net/mptcp/pm_netlink.c
>>>> @@ -1025,7 +1025,7 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
>>>> }
>>>>
>>>> *entry = addr;
>>>> - if (entry->addr.port) {
>>>> + if (entry->addr.port && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
>>>> ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
>>>> if (ret) {
>>>> GENL_SET_ERR_MSG(info, "create listen socket error");
>>>> --
>>>> 1.8.3.1
>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 7:27 ` Geliang Tang
@ 2021-04-23 7:41 ` Jianguo Wu
2021-04-23 7:47 ` Geliang Tang
2021-04-23 8:21 ` Matthieu Baerts
0 siblings, 2 replies; 14+ messages in thread
From: Jianguo Wu @ 2021-04-23 7:41 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
On 2021/4/23 15:27, Geliang Tang wrote:
> Geliang Tang <geliangtang@gmail.com> 于2021年4月23日周五 下午3:17写道:
>>
>> Hi Jianguo,
>>
>> Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午2:39写道:
>>>
>>>
>>>
>>> On 2021/4/23 12:33, Geliang Tang wrote:
>>>> Hi Jianguo,
>>>>
>>>> Thanks for your patch.
>>>>
>>>> Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
>>>>>
>>>>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>>>>>
>>>>> PM announces address to remote host when signal flag is set,
>>>>> so create listening socket only if signal flag is set.
>>>>
>>>> This check has done in the user-space, here are these lines in pm_nl_ctl.c:
>>>>
>>>> 274 } else if (!strcmp(argv[arg], "port")) {
>>>> 275 u_int16_t port;
>>>> 276
>>>> 277 if (++arg >= argc)
>>>> 278 error(1, 0, " missing port value");
>>>> 279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
>>>> 280 error(1, 0, " flags must be signal
>>>> when using port");
>>>>
>>>
>>> Hi Geliang, Thanks for point out this, I was use 'ip mptcp' to add address, It doesn't seem to have done that check.
>>> Is it worth to do check in kernel to make sure there is a signal flag before create listening socket?
>>
>> I think it's better to check this in the user-space, you can write a patch
>> to iproute2 to add this check in 'ip mptcp'.
>
> Or add this check in mptcp_pm_parse_addr, something like:
>
> $ git diff net/mptcp/pm_netlink.c
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 6ba040897738..9cd4a4180a2a 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -971,8 +971,14 @@ static int mptcp_pm_parse_addr(struct nlattr
> *attr, struct genl_info *info,
> if (tb[MPTCP_PM_ADDR_ATTR_FLAGS])
> entry->flags = nla_get_u32(tb[MPTCP_PM_ADDR_ATTR_FLAGS]);
>
> - if (tb[MPTCP_PM_ADDR_ATTR_PORT])
> + if (tb[MPTCP_PM_ADDR_ATTR_PORT]) {
> + if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> + NL_SET_ERR_MSG_ATTR("");
> + return -EINVAL;
> + }
> +
> entry->addr.port =
> htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]));
> + }
>
> return 0;
> }
>
> WDYT?
>
I think this is better, we can't assume that all user-space tools works correctly.
> -Geliang
>
>
>>
>> Thanks.
>>
>> -Geliang
>>
>>>
>>>> -Geliang
>>>>
>>>>>
>>>>> Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
>>>>> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
>>>>> ---
>>>>> net/mptcp/pm_netlink.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>>>>> index becc32b..dbd6910 100644
>>>>> --- a/net/mptcp/pm_netlink.c
>>>>> +++ b/net/mptcp/pm_netlink.c
>>>>> @@ -1025,7 +1025,7 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
>>>>> }
>>>>>
>>>>> *entry = addr;
>>>>> - if (entry->addr.port) {
>>>>> + if (entry->addr.port && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
>>>>> ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
>>>>> if (ret) {
>>>>> GENL_SET_ERR_MSG(info, "create listen socket error");
>>>>> --
>>>>> 1.8.3.1
>>>>>
>>>>>
>>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 7:41 ` Jianguo Wu
@ 2021-04-23 7:47 ` Geliang Tang
2021-04-23 8:08 ` Jianguo Wu
2021-04-23 8:21 ` Matthieu Baerts
1 sibling, 1 reply; 14+ messages in thread
From: Geliang Tang @ 2021-04-23 7:47 UTC (permalink / raw)
To: Jianguo Wu; +Cc: mptcp
Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午3:41写道:
>
>
>
> On 2021/4/23 15:27, Geliang Tang wrote:
> > Geliang Tang <geliangtang@gmail.com> 于2021年4月23日周五 下午3:17写道:
> >>
> >> Hi Jianguo,
> >>
> >> Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午2:39写道:
> >>>
> >>>
> >>>
> >>> On 2021/4/23 12:33, Geliang Tang wrote:
> >>>> Hi Jianguo,
> >>>>
> >>>> Thanks for your patch.
> >>>>
> >>>> Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
> >>>>>
> >>>>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
> >>>>>
> >>>>> PM announces address to remote host when signal flag is set,
> >>>>> so create listening socket only if signal flag is set.
> >>>>
> >>>> This check has done in the user-space, here are these lines in pm_nl_ctl.c:
> >>>>
> >>>> 274 } else if (!strcmp(argv[arg], "port")) {
> >>>> 275 u_int16_t port;
> >>>> 276
> >>>> 277 if (++arg >= argc)
> >>>> 278 error(1, 0, " missing port value");
> >>>> 279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> >>>> 280 error(1, 0, " flags must be signal
> >>>> when using port");
> >>>>
> >>>
> >>> Hi Geliang, Thanks for point out this, I was use 'ip mptcp' to add address, It doesn't seem to have done that check.
> >>> Is it worth to do check in kernel to make sure there is a signal flag before create listening socket?
> >>
> >> I think it's better to check this in the user-space, you can write a patch
> >> to iproute2 to add this check in 'ip mptcp'.
> >
> > Or add this check in mptcp_pm_parse_addr, something like:
> >
> > $ git diff net/mptcp/pm_netlink.c
> > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > index 6ba040897738..9cd4a4180a2a 100644
> > --- a/net/mptcp/pm_netlink.c
> > +++ b/net/mptcp/pm_netlink.c
> > @@ -971,8 +971,14 @@ static int mptcp_pm_parse_addr(struct nlattr
> > *attr, struct genl_info *info,
> > if (tb[MPTCP_PM_ADDR_ATTR_FLAGS])
> > entry->flags = nla_get_u32(tb[MPTCP_PM_ADDR_ATTR_FLAGS]);
> >
> > - if (tb[MPTCP_PM_ADDR_ATTR_PORT])
> > + if (tb[MPTCP_PM_ADDR_ATTR_PORT]) {
> > + if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> > + NL_SET_ERR_MSG_ATTR("");
> > + return -EINVAL;
> > + }
> > +
> > entry->addr.port =
> > htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]));
> > + }
> >
> > return 0;
> > }
> >
> > WDYT?
> >
>
> I think this is better, we can't assume that all user-space tools works correctly.
You can add this check both in mptcp_pm_parse_addr and in 'ip mptcp'.
-Geliang
>
> > -Geliang
> >
> >
> >>
> >> Thanks.
> >>
> >> -Geliang
> >>
> >>>
> >>>> -Geliang
> >>>>
> >>>>>
> >>>>> Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
> >>>>> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
> >>>>> ---
> >>>>> net/mptcp/pm_netlink.c | 2 +-
> >>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> >>>>> index becc32b..dbd6910 100644
> >>>>> --- a/net/mptcp/pm_netlink.c
> >>>>> +++ b/net/mptcp/pm_netlink.c
> >>>>> @@ -1025,7 +1025,7 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
> >>>>> }
> >>>>>
> >>>>> *entry = addr;
> >>>>> - if (entry->addr.port) {
> >>>>> + if (entry->addr.port && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> >>>>> ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
> >>>>> if (ret) {
> >>>>> GENL_SET_ERR_MSG(info, "create listen socket error");
> >>>>> --
> >>>>> 1.8.3.1
> >>>>>
> >>>>>
> >>>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 7:47 ` Geliang Tang
@ 2021-04-23 8:08 ` Jianguo Wu
0 siblings, 0 replies; 14+ messages in thread
From: Jianguo Wu @ 2021-04-23 8:08 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
On 2021/4/23 15:47, Geliang Tang wrote:
> Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午3:41写道:
>>
>>
>>
>> On 2021/4/23 15:27, Geliang Tang wrote:
>>> Geliang Tang <geliangtang@gmail.com> 于2021年4月23日周五 下午3:17写道:
>>>>
>>>> Hi Jianguo,
>>>>
>>>> Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午2:39写道:
>>>>>
>>>>>
>>>>>
>>>>> On 2021/4/23 12:33, Geliang Tang wrote:
>>>>>> Hi Jianguo,
>>>>>>
>>>>>> Thanks for your patch.
>>>>>>
>>>>>> Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
>>>>>>>
>>>>>>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>>>>>>>
>>>>>>> PM announces address to remote host when signal flag is set,
>>>>>>> so create listening socket only if signal flag is set.
>>>>>>
>>>>>> This check has done in the user-space, here are these lines in pm_nl_ctl.c:
>>>>>>
>>>>>> 274 } else if (!strcmp(argv[arg], "port")) {
>>>>>> 275 u_int16_t port;
>>>>>> 276
>>>>>> 277 if (++arg >= argc)
>>>>>> 278 error(1, 0, " missing port value");
>>>>>> 279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
>>>>>> 280 error(1, 0, " flags must be signal
>>>>>> when using port");
>>>>>>
>>>>>
>>>>> Hi Geliang, Thanks for point out this, I was use 'ip mptcp' to add address, It doesn't seem to have done that check.
>>>>> Is it worth to do check in kernel to make sure there is a signal flag before create listening socket?
>>>>
>>>> I think it's better to check this in the user-space, you can write a patch
>>>> to iproute2 to add this check in 'ip mptcp'.
>>>
>>> Or add this check in mptcp_pm_parse_addr, something like:
>>>
>>> $ git diff net/mptcp/pm_netlink.c
>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>>> index 6ba040897738..9cd4a4180a2a 100644
>>> --- a/net/mptcp/pm_netlink.c
>>> +++ b/net/mptcp/pm_netlink.c
>>> @@ -971,8 +971,14 @@ static int mptcp_pm_parse_addr(struct nlattr
>>> *attr, struct genl_info *info,
>>> if (tb[MPTCP_PM_ADDR_ATTR_FLAGS])
>>> entry->flags = nla_get_u32(tb[MPTCP_PM_ADDR_ATTR_FLAGS]);
>>>
>>> - if (tb[MPTCP_PM_ADDR_ATTR_PORT])
>>> + if (tb[MPTCP_PM_ADDR_ATTR_PORT]) {
>>> + if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
>>> + NL_SET_ERR_MSG_ATTR("");
>>> + return -EINVAL;
>>> + }
>>> +
>>> entry->addr.port =
>>> htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]));
>>> + }
>>>
>>> return 0;
>>> }
>>>
>>> WDYT?
>>>
>>
>> I think this is better, we can't assume that all user-space tools works correctly.
>
> You can add this check both in mptcp_pm_parse_addr and in 'ip mptcp'.
>
Ok, I will send patches soon, Thanks:)
> -Geliang
>
>>
>>> -Geliang
>>>
>>>
>>>>
>>>> Thanks.
>>>>
>>>> -Geliang
>>>>
>>>>>
>>>>>> -Geliang
>>>>>>
>>>>>>>
>>>>>>> Fixes: 1729cf186d8a ("mptcp: create the listening socket for new port")
>>>>>>> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
>>>>>>> ---
>>>>>>> net/mptcp/pm_netlink.c | 2 +-
>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>>>>>>> index becc32b..dbd6910 100644
>>>>>>> --- a/net/mptcp/pm_netlink.c
>>>>>>> +++ b/net/mptcp/pm_netlink.c
>>>>>>> @@ -1025,7 +1025,7 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
>>>>>>> }
>>>>>>>
>>>>>>> *entry = addr;
>>>>>>> - if (entry->addr.port) {
>>>>>>> + if (entry->addr.port && (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
>>>>>>> ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
>>>>>>> if (ret) {
>>>>>>> GENL_SET_ERR_MSG(info, "create listen socket error");
>>>>>>> --
>>>>>>> 1.8.3.1
>>>>>>>
>>>>>>>
>>>>>
>>
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 7:41 ` Jianguo Wu
2021-04-23 7:47 ` Geliang Tang
@ 2021-04-23 8:21 ` Matthieu Baerts
2021-04-23 8:38 ` Jianguo Wu
2021-04-23 8:59 ` Geliang Tang
1 sibling, 2 replies; 14+ messages in thread
From: Matthieu Baerts @ 2021-04-23 8:21 UTC (permalink / raw)
To: Jianguo Wu, Geliang Tang; +Cc: mptcp
Hi Jianguo, Geliang,
Thank you for the patches and reviews!
On 23/04/2021 09:41, Jianguo Wu wrote:
>
>
> On 2021/4/23 15:27, Geliang Tang wrote:
>> Geliang Tang <geliangtang@gmail.com> 于2021年4月23日周五 下午3:17写道:
>>>
>>> Hi Jianguo,
>>>
>>> Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午2:39写道:
>>>>
>>>>
>>>>
>>>> On 2021/4/23 12:33, Geliang Tang wrote:
>>>>> Hi Jianguo,
>>>>>
>>>>> Thanks for your patch.
>>>>>
>>>>> Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
>>>>>>
>>>>>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>>>>>>
>>>>>> PM announces address to remote host when signal flag is set,
>>>>>> so create listening socket only if signal flag is set.
>>>>>
>>>>> This check has done in the user-space, here are these lines in pm_nl_ctl.c:
>>>>>
>>>>> 274 } else if (!strcmp(argv[arg], "port")) {
>>>>> 275 u_int16_t port;
>>>>> 276
>>>>> 277 if (++arg >= argc)
>>>>> 278 error(1, 0, " missing port value");
>>>>> 279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
>>>>> 280 error(1, 0, " flags must be signal
>>>>> when using port");
>>>>>
>>>>
>>>> Hi Geliang, Thanks for point out this, I was use 'ip mptcp' to add address, It doesn't seem to have done that check.
>>>> Is it worth to do check in kernel to make sure there is a signal flag before create listening socket?
>>>
>>> I think it's better to check this in the user-space, you can write a patch
>>> to iproute2 to add this check in 'ip mptcp'.
>>
>> Or add this check in mptcp_pm_parse_addr, something like:
>>
>> $ git diff net/mptcp/pm_netlink.c
>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>> index 6ba040897738..9cd4a4180a2a 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c
>> @@ -971,8 +971,14 @@ static int mptcp_pm_parse_addr(struct nlattr
>> *attr, struct genl_info *info,
>> if (tb[MPTCP_PM_ADDR_ATTR_FLAGS])
>> entry->flags = nla_get_u32(tb[MPTCP_PM_ADDR_ATTR_FLAGS]);
>>
>> - if (tb[MPTCP_PM_ADDR_ATTR_PORT])
>> + if (tb[MPTCP_PM_ADDR_ATTR_PORT]) {
>> + if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
>> + NL_SET_ERR_MSG_ATTR("");
>> + return -EINVAL;
>> + }
>> +
>> entry->addr.port =
>> htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]));
>> + }
>>
>> return 0;
>> }
>>
>> WDYT?
>>
>
> I think this is better, we can't assume that all user-space tools works correctly.
I was thinking a bit about that. With the proposed modifications, it
means that a server will have to send an ADD_ADDR if it wants to accept
subflows on another port. I guess there will be use-cases where we will
not want to send ADD_ADDR because we are in a controlled environment or
we don't want to announce a public ADD_ADDR+port on a specific network.
Do we have to support that? Maybe safer to restrict actions for now and
allow that later if there are needs, no?
In other words, I guess we should accept the proposed modifications.
Any other points of view? :)
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 8:21 ` Matthieu Baerts
@ 2021-04-23 8:38 ` Jianguo Wu
2021-04-23 14:02 ` Matthieu Baerts
2021-04-23 8:59 ` Geliang Tang
1 sibling, 1 reply; 14+ messages in thread
From: Jianguo Wu @ 2021-04-23 8:38 UTC (permalink / raw)
To: Matthieu Baerts, Geliang Tang; +Cc: mptcp
On 2021/4/23 16:21, Matthieu Baerts wrote:
> Hi Jianguo, Geliang,
>
> Thank you for the patches and reviews!
>
> On 23/04/2021 09:41, Jianguo Wu wrote:
>>
>>
>> On 2021/4/23 15:27, Geliang Tang wrote:
>>> Geliang Tang <geliangtang@gmail.com> 于2021年4月23日周五 下午3:17写道:
>>>>
>>>> Hi Jianguo,
>>>>
>>>> Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午2:39写道:
>>>>>
>>>>>
>>>>>
>>>>> On 2021/4/23 12:33, Geliang Tang wrote:
>>>>>> Hi Jianguo,
>>>>>>
>>>>>> Thanks for your patch.
>>>>>>
>>>>>> Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
>>>>>>>
>>>>>>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>>>>>>>
>>>>>>> PM announces address to remote host when signal flag is set,
>>>>>>> so create listening socket only if signal flag is set.
>>>>>>
>>>>>> This check has done in the user-space, here are these lines in pm_nl_ctl.c:
>>>>>>
>>>>>> 274 } else if (!strcmp(argv[arg], "port")) {
>>>>>> 275 u_int16_t port;
>>>>>> 276
>>>>>> 277 if (++arg >= argc)
>>>>>> 278 error(1, 0, " missing port value");
>>>>>> 279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
>>>>>> 280 error(1, 0, " flags must be signal
>>>>>> when using port");
>>>>>>
>>>>>
>>>>> Hi Geliang, Thanks for point out this, I was use 'ip mptcp' to add address, It doesn't seem to have done that check.
>>>>> Is it worth to do check in kernel to make sure there is a signal flag before create listening socket?
>>>>
>>>> I think it's better to check this in the user-space, you can write a patch
>>>> to iproute2 to add this check in 'ip mptcp'.
>>>
>>> Or add this check in mptcp_pm_parse_addr, something like:
>>>
>>> $ git diff net/mptcp/pm_netlink.c
>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>>> index 6ba040897738..9cd4a4180a2a 100644
>>> --- a/net/mptcp/pm_netlink.c
>>> +++ b/net/mptcp/pm_netlink.c
>>> @@ -971,8 +971,14 @@ static int mptcp_pm_parse_addr(struct nlattr
>>> *attr, struct genl_info *info,
>>> if (tb[MPTCP_PM_ADDR_ATTR_FLAGS])
>>> entry->flags = nla_get_u32(tb[MPTCP_PM_ADDR_ATTR_FLAGS]);
>>>
>>> - if (tb[MPTCP_PM_ADDR_ATTR_PORT])
>>> + if (tb[MPTCP_PM_ADDR_ATTR_PORT]) {
>>> + if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
>>> + NL_SET_ERR_MSG_ATTR("");
>>> + return -EINVAL;
>>> + }
>>> +
>>> entry->addr.port =
>>> htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]));
>>> + }
>>>
>>> return 0;
>>> }
>>>
>>> WDYT?
>>>
>>
>> I think this is better, we can't assume that all user-space tools works correctly.
>
> I was thinking a bit about that. With the proposed modifications, it
> means that a server will have to send an ADD_ADDR if it wants to accept
> subflows on another port. I guess there will be use-cases where we will
> not want to send ADD_ADDR because we are in a controlled environment or
> we don't want to announce a public ADD_ADDR+port on a specific network.
>
Hi Matt, do you mean there will be use-cases where using ADD_ADDR+port as local address to initiate
a outgoing subflow join, but not send ADD_ADDR?
> Do we have to support that? Maybe safer to restrict actions for now and
> allow that later if there are needs, no?
>
> In other words, I guess we should accept the proposed modifications.
> Any other points of view? :)
>
> Cheers,
> Matt
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 8:21 ` Matthieu Baerts
2021-04-23 8:38 ` Jianguo Wu
@ 2021-04-23 8:59 ` Geliang Tang
2021-04-23 14:04 ` Matthieu Baerts
1 sibling, 1 reply; 14+ messages in thread
From: Geliang Tang @ 2021-04-23 8:59 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: Jianguo Wu, mptcp
Hi Matt,
Matthieu Baerts <matthieu.baerts@tessares.net> 于2021年4月23日周五 下午4:21写道:
>
> Hi Jianguo, Geliang,
>
> Thank you for the patches and reviews!
>
> On 23/04/2021 09:41, Jianguo Wu wrote:
> >
> >
> > On 2021/4/23 15:27, Geliang Tang wrote:
> >> Geliang Tang <geliangtang@gmail.com> 于2021年4月23日周五 下午3:17写道:
> >>>
> >>> Hi Jianguo,
> >>>
> >>> Jianguo Wu <wujianguo106@163.com> 于2021年4月23日周五 下午2:39写道:
> >>>>
> >>>>
> >>>>
> >>>> On 2021/4/23 12:33, Geliang Tang wrote:
> >>>>> Hi Jianguo,
> >>>>>
> >>>>> Thanks for your patch.
> >>>>>
> >>>>> Jianguo Wu <wujianguo106@163.com> 于2021年4月22日周四 下午6:30写道:
> >>>>>>
> >>>>>> From: Jianguo Wu <wujianguo@chinatelecom.cn>
> >>>>>>
> >>>>>> PM announces address to remote host when signal flag is set,
> >>>>>> so create listening socket only if signal flag is set.
> >>>>>
> >>>>> This check has done in the user-space, here are these lines in pm_nl_ctl.c:
> >>>>>
> >>>>> 274 } else if (!strcmp(argv[arg], "port")) {
> >>>>> 275 u_int16_t port;
> >>>>> 276
> >>>>> 277 if (++arg >= argc)
> >>>>> 278 error(1, 0, " missing port value");
> >>>>> 279 if (!(flags & MPTCP_PM_ADDR_FLAG_SIGNAL))
> >>>>> 280 error(1, 0, " flags must be signal
> >>>>> when using port");
> >>>>>
> >>>>
> >>>> Hi Geliang, Thanks for point out this, I was use 'ip mptcp' to add address, It doesn't seem to have done that check.
> >>>> Is it worth to do check in kernel to make sure there is a signal flag before create listening socket?
> >>>
> >>> I think it's better to check this in the user-space, you can write a patch
> >>> to iproute2 to add this check in 'ip mptcp'.
> >>
> >> Or add this check in mptcp_pm_parse_addr, something like:
> >>
> >> $ git diff net/mptcp/pm_netlink.c
> >> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> >> index 6ba040897738..9cd4a4180a2a 100644
> >> --- a/net/mptcp/pm_netlink.c
> >> +++ b/net/mptcp/pm_netlink.c
> >> @@ -971,8 +971,14 @@ static int mptcp_pm_parse_addr(struct nlattr
> >> *attr, struct genl_info *info,
> >> if (tb[MPTCP_PM_ADDR_ATTR_FLAGS])
> >> entry->flags = nla_get_u32(tb[MPTCP_PM_ADDR_ATTR_FLAGS]);
> >>
> >> - if (tb[MPTCP_PM_ADDR_ATTR_PORT])
> >> + if (tb[MPTCP_PM_ADDR_ATTR_PORT]) {
> >> + if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL)) {
> >> + NL_SET_ERR_MSG_ATTR("");
> >> + return -EINVAL;
> >> + }
> >> +
> >> entry->addr.port =
> >> htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]));
> >> + }
> >>
> >> return 0;
> >> }
> >>
> >> WDYT?
> >>
> >
> > I think this is better, we can't assume that all user-space tools works correctly.
>
> I was thinking a bit about that. With the proposed modifications, it
> means that a server will have to send an ADD_ADDR if it wants to accept
> subflows on another port. I guess there will be use-cases where we will
> not want to send ADD_ADDR because we are in a controlled environment or
> we don't want to announce a public ADD_ADDR+port on a specific network.
>
> Do we have to support that? Maybe safer to restrict actions for now and
> allow that later if there are needs, no?
>
> In other words, I guess we should accept the proposed modifications.
> Any other points of view? :)
I think the proposed modifications is not enough.
If the port is set with non-signal flag (like subflow), we should not only
stop creating listening socket, but also stop appending this address.
Since our code can't deal with the ADD_SUBFLOW+port case (like
"pm_nl_ctl add 10.0.0.1 flag subflow port 1000"). Local address's port will
re-set as the main socket's port number in our code. So set port to a local
address is useless, we should add the flag check to avoid this case.
I think the flag check should be added both in mptcp_pm_parse_addr and in
'ip mptcp'. Let's waiting for Jianguo's new patches.
Thanks.
-Geliang
>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 8:38 ` Jianguo Wu
@ 2021-04-23 14:02 ` Matthieu Baerts
0 siblings, 0 replies; 14+ messages in thread
From: Matthieu Baerts @ 2021-04-23 14:02 UTC (permalink / raw)
To: Jianguo Wu, Geliang Tang; +Cc: mptcp
Hi Jianguo,
On 23/04/2021 10:38, Jianguo Wu wrote:
> On 2021/4/23 16:21, Matthieu Baerts wrote:
>> On 23/04/2021 09:41, Jianguo Wu wrote:
>>> I think this is better, we can't assume that all user-space tools works correctly.
>>
>> I was thinking a bit about that. With the proposed modifications, it
>> means that a server will have to send an ADD_ADDR if it wants to accept
>> subflows on another port. I guess there will be use-cases where we will
>> not want to send ADD_ADDR because we are in a controlled environment or
>> we don't want to announce a public ADD_ADDR+port on a specific network.
>>
>
> Hi Matt, do you mean there will be use-cases where using ADD_ADDR+port as local address to initiate
> a outgoing subflow join, but not send ADD_ADDR?
That would definitively not be a common use-case but we will see if some
people are interested by that.
Anyhow, I think it is better to start by well covering requested
use-cases and avoid "strange" behaviours, e.g. not announcing the
address but doing bind.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] mptcp: create listening socket only if signal flag is set on
2021-04-23 8:59 ` Geliang Tang
@ 2021-04-23 14:04 ` Matthieu Baerts
0 siblings, 0 replies; 14+ messages in thread
From: Matthieu Baerts @ 2021-04-23 14:04 UTC (permalink / raw)
To: Geliang Tang; +Cc: Jianguo Wu, mptcp
Hi Geliang,
On 23/04/2021 10:59, Geliang Tang wrote:
> Matthieu Baerts <matthieu.baerts@tessares.net> 于2021年4月23日周五 下午4:21写道:
>> On 23/04/2021 09:41, Jianguo Wu wrote:
>>> I think this is better, we can't assume that all user-space tools works correctly.
>>
>> I was thinking a bit about that. With the proposed modifications, it
>> means that a server will have to send an ADD_ADDR if it wants to accept
>> subflows on another port. I guess there will be use-cases where we will
>> not want to send ADD_ADDR because we are in a controlled environment or
>> we don't want to announce a public ADD_ADDR+port on a specific network.
>>
>> Do we have to support that? Maybe safer to restrict actions for now and
>> allow that later if there are needs, no?
>>
>> In other words, I guess we should accept the proposed modifications.
>> Any other points of view? :)
>
> I think the proposed modifications is not enough.
>
> If the port is set with non-signal flag (like subflow), we should not only
> stop creating listening socket, but also stop appending this address.
>
> Since our code can't deal with the ADD_SUBFLOW+port case (like
> "pm_nl_ctl add 10.0.0.1 flag subflow port 1000"). Local address's port will
> re-set as the main socket's port number in our code. So set port to a local
> address is useless, we should add the flag check to avoid this case.
Indeed, that seems to make sense!
Thanks for checking this!
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-04-23 14:04 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 10:30 [PATCH] mptcp: create listening socket only if signal flag is set on Jianguo Wu
2021-04-23 4:33 ` Geliang Tang
2021-04-23 6:38 ` Jianguo Wu
2021-04-23 7:17 ` Geliang Tang
2021-04-23 7:27 ` Geliang Tang
2021-04-23 7:41 ` Jianguo Wu
2021-04-23 7:47 ` Geliang Tang
2021-04-23 8:08 ` Jianguo Wu
2021-04-23 8:21 ` Matthieu Baerts
2021-04-23 8:38 ` Jianguo Wu
2021-04-23 14:02 ` Matthieu Baerts
2021-04-23 8:59 ` Geliang Tang
2021-04-23 14:04 ` Matthieu Baerts
2021-04-23 7:28 ` Jianguo Wu
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.