Per RFC 8684, if no port is specified in an ADD_ADDR message, MPTCP
SHOULD attempt to connect to the specified address on the same port
as the port that is already in use by the subflow on which the
ADD_ADDR signal was sent.
To facilitate that, this change reflects the specific remote port in
use by that subflow in MPTCP_EVENT_ANNOUNCED events.
Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
---
net/mptcp/options.c | 2 +-
net/mptcp/pm.c | 5 +++--
net/mptcp/pm_netlink.c | 8 ++++++--
net/mptcp/protocol.h | 6 ++++--
4 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 0d0d2eb8c8ca..0d3c8f7e5be6 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1132,7 +1132,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
if ((mp_opt.suboptions & OPTION_MPTCP_ADD_ADDR) &&
add_addr_hmac_valid(msk, &mp_opt)) {
if (!mp_opt.echo) {
- mptcp_pm_add_addr_received(msk, &mp_opt.addr);
+ mptcp_pm_add_addr_received(msk, &mp_opt.addr, sk);
MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDR);
} else {
mptcp_pm_add_addr_echoed(msk, &mp_opt.addr);
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 6b6220895929..e5d5cb847209 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -200,14 +200,15 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk,
}
void mptcp_pm_add_addr_received(struct mptcp_sock *msk,
- const struct mptcp_addr_info *addr)
+ const struct mptcp_addr_info *addr,
+ const struct sock *ssk)
{
struct mptcp_pm_data *pm = &msk->pm;
pr_debug("msk=%p remote_id=%d accept=%d", msk, addr->id,
READ_ONCE(pm->accept_addr));
- mptcp_event_addr_announced(msk, addr);
+ mptcp_event_addr_announced(msk, addr, ssk);
spin_lock_bh(&pm->lock);
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 93800f32fcb6..f90e77c3775d 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1972,7 +1972,8 @@ void mptcp_event_addr_removed(const struct mptcp_sock *msk, uint8_t id)
}
void mptcp_event_addr_announced(const struct mptcp_sock *msk,
- const struct mptcp_addr_info *info)
+ const struct mptcp_addr_info *info,
+ const struct sock *ssk)
{
struct net *net = sock_net((const struct sock *)msk);
struct nlmsghdr *nlh;
@@ -1996,7 +1997,10 @@ void mptcp_event_addr_announced(const struct mptcp_sock *msk,
if (nla_put_u8(skb, MPTCP_ATTR_REM_ID, info->id))
goto nla_put_failure;
- if (nla_put_be16(skb, MPTCP_ATTR_DPORT, info->port))
+ if (nla_put_be16(skb, MPTCP_ATTR_DPORT,
+ info->port == 0 ?
+ ((struct inet_sock *)inet_sk(ssk))->inet_dport :
+ info->port))
goto nla_put_failure;
switch (info->family) {
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index c47d69a42fcb..d20c65fcba89 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -753,7 +753,8 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk);
void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk,
const struct mptcp_subflow_context *subflow);
void mptcp_pm_add_addr_received(struct mptcp_sock *msk,
- const struct mptcp_addr_info *addr);
+ const struct mptcp_addr_info *addr,
+ const struct sock *ssk);
void mptcp_pm_add_addr_echoed(struct mptcp_sock *msk,
struct mptcp_addr_info *addr);
void mptcp_pm_add_addr_send_ack(struct mptcp_sock *msk);
@@ -781,7 +782,8 @@ int mptcp_pm_remove_subflow(struct mptcp_sock *msk, const struct mptcp_rm_list *
void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
const struct sock *ssk, gfp_t gfp);
-void mptcp_event_addr_announced(const struct mptcp_sock *msk, const struct mptcp_addr_info *info);
+void mptcp_event_addr_announced(const struct mptcp_sock *msk, const struct mptcp_addr_info *info,
+ const struct sock *ssk);
void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
--
2.31.1
Hi Kishen,
Kishen Maloor <kishen.maloor@intel.com> 于2022年1月28日周五 08:38写道：
>
> Per RFC 8684, if no port is specified in an ADD_ADDR message, MPTCP
> SHOULD attempt to connect to the specified address on the same port
> as the port that is already in use by the subflow on which the
> ADD_ADDR signal was sent.
>
> To facilitate that, this change reflects the specific remote port in
> use by that subflow in MPTCP_EVENT_ANNOUNCED events.
>
> Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
> ---
> net/mptcp/options.c | 2 +-
> net/mptcp/pm.c | 5 +++--
> net/mptcp/pm_netlink.c | 8 ++++++--
> net/mptcp/protocol.h | 6 ++++--
> 4 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 0d0d2eb8c8ca..0d3c8f7e5be6 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1132,7 +1132,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
> if ((mp_opt.suboptions & OPTION_MPTCP_ADD_ADDR) &&
> add_addr_hmac_valid(msk, &mp_opt)) {
> if (!mp_opt.echo) {
> - mptcp_pm_add_addr_received(msk, &mp_opt.addr);
> + mptcp_pm_add_addr_received(msk, &mp_opt.addr, sk);
> MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDR);
> } else {
> mptcp_pm_add_addr_echoed(msk, &mp_opt.addr);
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 6b6220895929..e5d5cb847209 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -200,14 +200,15 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk,
> }
>
> void mptcp_pm_add_addr_received(struct mptcp_sock *msk,
> - const struct mptcp_addr_info *addr)
> + const struct mptcp_addr_info *addr,
> + const struct sock *ssk)
I think the better parameters order of this function is:
(struct mptcp_sock *msk, const struct sock *ssk, const struct
mptcp_addr_info *addr)
Put the new parameter ssk just after msk.
Furthermore, I think instead of adding a new parameters ssk here, it's
better to change the parameter msk to ssk.
We can get the msk from ssk like this:
struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
struct mptcp_sock *msk = mptcp_sk(subflow->conn);
> {
> struct mptcp_pm_data *pm = &msk->pm;
>
> pr_debug("msk=%p remote_id=%d accept=%d", msk, addr->id,
> READ_ONCE(pm->accept_addr));
>
> - mptcp_event_addr_announced(msk, addr);
> + mptcp_event_addr_announced(msk, addr, ssk);
>
> spin_lock_bh(&pm->lock);
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 93800f32fcb6..f90e77c3775d 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1972,7 +1972,8 @@ void mptcp_event_addr_removed(const struct mptcp_sock *msk, uint8_t id)
> }
>
> void mptcp_event_addr_announced(const struct mptcp_sock *msk,
> - const struct mptcp_addr_info *info)
> + const struct mptcp_addr_info *info,
> + const struct sock *ssk)
Here change the parameter msk to ssk too.
> {
> struct net *net = sock_net((const struct sock *)msk);
struct net *net = sock_net(ssk);
> struct nlmsghdr *nlh;
> @@ -1996,7 +1997,10 @@ void mptcp_event_addr_announced(const struct mptcp_sock *msk,
> if (nla_put_u8(skb, MPTCP_ATTR_REM_ID, info->id))
> goto nla_put_failure;
>
> - if (nla_put_be16(skb, MPTCP_ATTR_DPORT, info->port))
> + if (nla_put_be16(skb, MPTCP_ATTR_DPORT,
> + info->port == 0 ?
Here two blanks before '=='.
Thanks,
-Geliang
> + ((struct inet_sock *)inet_sk(ssk))->inet_dport :
> + info->port))
> goto nla_put_failure;
>
> switch (info->family) {
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index c47d69a42fcb..d20c65fcba89 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -753,7 +753,8 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk);
> void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk,
> const struct mptcp_subflow_context *subflow);
> void mptcp_pm_add_addr_received(struct mptcp_sock *msk,
> - const struct mptcp_addr_info *addr);
> + const struct mptcp_addr_info *addr,
> + const struct sock *ssk);
> void mptcp_pm_add_addr_echoed(struct mptcp_sock *msk,
> struct mptcp_addr_info *addr);
> void mptcp_pm_add_addr_send_ack(struct mptcp_sock *msk);
> @@ -781,7 +782,8 @@ int mptcp_pm_remove_subflow(struct mptcp_sock *msk, const struct mptcp_rm_list *
>
> void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
> const struct sock *ssk, gfp_t gfp);
> -void mptcp_event_addr_announced(const struct mptcp_sock *msk, const struct mptcp_addr_info *info);
> +void mptcp_event_addr_announced(const struct mptcp_sock *msk, const struct mptcp_addr_info *info,
> + const struct sock *ssk);
> void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
>
> static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
> --
> 2.31.1
>
>
On 1/27/22 8:07 PM, Geliang Tang wrote:
> Hi Kishen,
>
> Kishen Maloor <kishen.maloor@intel.com> 于2022年1月28日周五 08:38写道：
>>
>> Per RFC 8684, if no port is specified in an ADD_ADDR message, MPTCP
>> SHOULD attempt to connect to the specified address on the same port
>> as the port that is already in use by the subflow on which the
>> ADD_ADDR signal was sent.
>>
>> To facilitate that, this change reflects the specific remote port in
>> use by that subflow in MPTCP_EVENT_ANNOUNCED events.
>>
>> Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
>> ---
>> net/mptcp/options.c | 2 +-
>> net/mptcp/pm.c | 5 +++--
>> net/mptcp/pm_netlink.c | 8 ++++++--
>> net/mptcp/protocol.h | 6 ++++--
>> 4 files changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>> index 0d0d2eb8c8ca..0d3c8f7e5be6 100644
>> --- a/net/mptcp/options.c
>> +++ b/net/mptcp/options.c
>> @@ -1132,7 +1132,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
>> if ((mp_opt.suboptions & OPTION_MPTCP_ADD_ADDR) &&
>> add_addr_hmac_valid(msk, &mp_opt)) {
>> if (!mp_opt.echo) {
>> - mptcp_pm_add_addr_received(msk, &mp_opt.addr);
>> + mptcp_pm_add_addr_received(msk, &mp_opt.addr, sk);
>> MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDR);
>> } else {
>> mptcp_pm_add_addr_echoed(msk, &mp_opt.addr);
>> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
>> index 6b6220895929..e5d5cb847209 100644
>> --- a/net/mptcp/pm.c
>> +++ b/net/mptcp/pm.c
>> @@ -200,14 +200,15 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk,
>> }
>>
>> void mptcp_pm_add_addr_received(struct mptcp_sock *msk,
>> - const struct mptcp_addr_info *addr)
>> + const struct mptcp_addr_info *addr,
>> + const struct sock *ssk)
>
> I think the better parameters order of this function is:
>
> (struct mptcp_sock *msk, const struct sock *ssk, const struct
> mptcp_addr_info *addr)
>
> Put the new parameter ssk just after msk.
>
> Furthermore, I think instead of adding a new parameters ssk here, it's
> better to change the parameter msk to ssk.
>
> We can get the msk from ssk like this:
>
> struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> struct mptcp_sock *msk = mptcp_sk(subflow->conn);
>
Thanks! Your suggestions look fine to me. I shall reflect this in v4.
>> {
>> struct mptcp_pm_data *pm = &msk->pm;
>>
>> pr_debug("msk=%p remote_id=%d accept=%d", msk, addr->id,
>> READ_ONCE(pm->accept_addr));
>>
>> - mptcp_event_addr_announced(msk, addr);
>> + mptcp_event_addr_announced(msk, addr, ssk);
>>
>> spin_lock_bh(&pm->lock);
>>
>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>> index 93800f32fcb6..f90e77c3775d 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c
>> @@ -1972,7 +1972,8 @@ void mptcp_event_addr_removed(const struct mptcp_sock *msk, uint8_t id)
>> }
>>
>> void mptcp_event_addr_announced(const struct mptcp_sock *msk,
>> - const struct mptcp_addr_info *info)
>> + const struct mptcp_addr_info *info,
>> + const struct sock *ssk)
>
> Here change the parameter msk to ssk too.
>
>> {
>> struct net *net = sock_net((const struct sock *)msk);
>
> struct net *net = sock_net(ssk);
>
>> struct nlmsghdr *nlh;
>> @@ -1996,7 +1997,10 @@ void mptcp_event_addr_announced(const struct mptcp_sock *msk,
>> if (nla_put_u8(skb, MPTCP_ATTR_REM_ID, info->id))
>> goto nla_put_failure;
>>
>> - if (nla_put_be16(skb, MPTCP_ATTR_DPORT, info->port))
>> + if (nla_put_be16(skb, MPTCP_ATTR_DPORT,
>> + info->port == 0 ?
>
> Here two blanks before '=='.
>
> Thanks,
> -Geliang
>
>> + ((struct inet_sock *)inet_sk(ssk))->inet_dport :
>> + info->port))
>> goto nla_put_failure;
>>
>> switch (info->family) {
>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>> index c47d69a42fcb..d20c65fcba89 100644
>> --- a/net/mptcp/protocol.h
>> +++ b/net/mptcp/protocol.h
>> @@ -753,7 +753,8 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk);
>> void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk,
>> const struct mptcp_subflow_context *subflow);
>> void mptcp_pm_add_addr_received(struct mptcp_sock *msk,
>> - const struct mptcp_addr_info *addr);
>> + const struct mptcp_addr_info *addr,
>> + const struct sock *ssk);
>> void mptcp_pm_add_addr_echoed(struct mptcp_sock *msk,
>> struct mptcp_addr_info *addr);
>> void mptcp_pm_add_addr_send_ack(struct mptcp_sock *msk);
>> @@ -781,7 +782,8 @@ int mptcp_pm_remove_subflow(struct mptcp_sock *msk, const struct mptcp_rm_list *
>>
>> void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
>> const struct sock *ssk, gfp_t gfp);
>> -void mptcp_event_addr_announced(const struct mptcp_sock *msk, const struct mptcp_addr_info *info);
>> +void mptcp_event_addr_announced(const struct mptcp_sock *msk, const struct mptcp_addr_info *info,
>> + const struct sock *ssk);
>> void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
>>
>> static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
>> --
>> 2.31.1
>>
>>
© 2016 - 2026 Red Hat, Inc.