* [MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 1/8] mptcp: create the listening socket for new port
@ 2020-12-19 0:23 Mat Martineau
0 siblings, 0 replies; 4+ messages in thread
From: Mat Martineau @ 2020-12-19 0:23 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 11416 bytes --]
On Sat, 19 Dec 2020, Geliang Tang wrote:
> Hi Mat,
>
> Mat Martineau <mathew.j.martineau(a)linux.intel.com> 于2020年12月18日周五 上午8:14写道：
>>
>> On Sun, 13 Dec 2020, Geliang Tang wrote:
>>
>>> This patch created a listening socket when an address with a port-number
>>> is added by PM netlink. Then binded the new port to the socket, and
>>> listened for the connection.
>>>
>>> Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
>>> ---
>>> net/mptcp/pm_netlink.c | 64 ++++++++++++++++++++++++++++++++++++++++++
>>> net/mptcp/protocol.c | 2 +-
>>> net/mptcp/protocol.h | 3 ++
>>> net/mptcp/subflow.c | 4 +--
>>> 4 files changed, 70 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>>> index 9b1f6298bbdb..1548efb22a1b 100644
>>> --- a/net/mptcp/pm_netlink.c
>>> +++ b/net/mptcp/pm_netlink.c
>>> @@ -26,6 +26,7 @@ struct mptcp_pm_addr_entry {
>>> struct list_head list;
>>> struct mptcp_addr_info addr;
>>> struct rcu_head rcu;
>>> + struct socket *lsk;
>>> };
>>>
>>> struct mptcp_pm_add_entry {
>>> @@ -613,6 +614,53 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
>>> return ret;
>>> }
>>>
>>> +static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
>>> + struct mptcp_pm_addr_entry *entry)
>>> +{
>>> + struct sockaddr_storage addr;
>>> + struct mptcp_sock *msk;
>>> + struct socket *ssock;
>>> + int backlog = 1024;
>>> + int err;
>>> +
>>> + err = sock_create_kern(sock_net(sk), entry->addr.family,
>>> + SOCK_STREAM, IPPROTO_MPTCP, &entry->lsk);
>>> + if (err)
>>> + return err;
>>> +
>>> + msk = mptcp_sk(entry->lsk->sk);
>>> + if (!msk) {
>>> + err = -EINVAL;
>>> + goto out;
>>> + }
>>> +
>>> + ssock = __mptcp_nmpc_socket(msk);
>>> + if (!ssock) {
>>> + err = -EINVAL;
>>> + goto out;
>>> + }
>>> +
>>> + mptcp_info2sockaddr(&entry->addr, &addr);
>>> + err = kernel_bind(ssock, (struct sockaddr *)&addr,
>>> + sizeof(struct sockaddr_in));
>>> + if (err) {
>>> + pr_warn("kernel_bind error, err=%d", err);
>>> + goto out;
>>> + }
>>> +
>>> + err = kernel_listen(ssock, backlog);
>>> + if (err) {
>>> + pr_warn("kernel_listen error, err=%d", err);
>>> + goto out;
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +out:
>>> + sock_release(entry->lsk);
>>> + return err;
>>> +}
>>> +
>>> int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
>>> {
>>> struct mptcp_pm_addr_entry *entry;
>>> @@ -657,6 +705,8 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
>>> entry->addr.ifindex = 0;
>>> entry->addr.flags = 0;
>>> entry->addr.id = 0;
>>> + entry->addr.port = 0;
>>> + entry->lsk = NULL;
>>> ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
>>> if (ret < 0)
>>> kfree(entry);
>>> @@ -808,9 +858,19 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
>>> }
>>>
>>> *entry = addr;
>>> + if (entry->addr.port) {
>>> + ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
>>> + if (ret) {
>>> + GENL_SET_ERR_MSG(info, "create listen socket error");
>>> + kfree(entry);
>>> + return ret;
>>> + }
>>> + }
>>> ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
>>> if (ret < 0) {
>>> GENL_SET_ERR_MSG(info, "too many addresses or duplicate one");
>>> + if (entry->lsk)
>>> + sock_release(entry->lsk);
>>> kfree(entry);
>>> return ret;
>>> }
>>> @@ -921,6 +981,8 @@ static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info)
>>> spin_unlock_bh(&pernet->lock);
>>>
>>> mptcp_nl_remove_subflow_and_signal_addr(sock_net(skb->sk), &entry->addr);
>>> + if (entry->lsk)
>>> + sock_release(entry->lsk);
>>> kfree_rcu(entry, rcu);
>>
>> Releasing the socket here could be racy - since the list is rcu-protected,
>> the entry could still be accessed (which is why it's freed with
>> kfree_rcu()). Rather than calling kfree_rcu(), use a custom callback with
>> call_rcu() that will both release lsk and kfree the list entry.
>>
>
> It dosen't work. I fixed it like this:
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 5f80b886aecb..d6b937bffb43 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1010,6 +1010,16 @@ static int
> mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
> return 0;
> }
>
> +static void mptcp_pm_addr_entry_free(struct rcu_head *head)
> +{
> + struct mptcp_pm_addr_entry *entry;
> +
> + entry = container_of(head, struct mptcp_pm_addr_entry, rcu);
> + if (entry->lsk)
> + sock_release(entry->lsk);
> + kfree(entry);
> +}
> +
> static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info)
> {
> struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
> @@ -1039,9 +1049,7 @@ static int mptcp_nl_cmd_del_addr(struct sk_buff
> *skb, struct genl_info *info)
> spin_unlock_bh(&pernet->lock);
>
> mptcp_nl_remove_subflow_and_signal_addr(sock_net(skb->sk), &entry->addr);
> - if (entry->lsk)
> - sock_release(entry->lsk);
> - kfree_rcu(entry, rcu);
> + call_rcu(&entry->rcu, mptcp_pm_addr_entry_free);
>
> return ret;
> }
> @@ -1054,10 +1062,8 @@ static void __flush_addrs(struct net *net,
> struct list_head *list)
> cur = list_entry(list->next,
> struct mptcp_pm_addr_entry, list);
> mptcp_nl_remove_subflow_and_signal_addr(net, &cur->addr);
> - if (cur->lsk)
> - sock_release(cur->lsk);
> list_del_rcu(&cur->list);
> - kfree_rcu(cur, rcu);
> + call_rcu(&cur->rcu, mptcp_pm_addr_entry_free);
> }
> }
>
> --
> 2.29.2
>
> But I got this error:
>
> [ 261.105979] MPTCP: sock_release 0000000058a9d993
> [ 261.105985] BUG: sleeping function called from invalid context at
> net/core/sock.c:3048
> [ 261.105987] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid:
> 0, name: swapper/2
> [ 261.105990] 1 lock held by swapper/2/0:
> [ 261.105992] #0: ffffffffac5a68e0 (rcu_callback){....}-{0:0}, at:
> rcu_do_batch+0x216/0x900
> [ 261.106009] CPU: 2 PID: 0 Comm: swapper/2 Kdump: loaded Not tainted
> 5.10.0-mptcp+ #429
> [ 261.106011] Hardware name: TIMI Mi Laptop Pro 15/TM1905, BIOS
> XMACM500P0301 04/08/2020
> [ 261.106013] Call Trace:
> [ 261.106016] <IRQ>
> [ 261.106022] dump_stack+0x8b/0xb0
> [ 261.106028] ___might_sleep.cold+0xb6/0xc6
> [ 261.106033] lock_sock_nested+0x28/0x90
> [ 261.106040] mptcp_close+0x20/0x2f0
> [ 261.106043] ? rcu_do_batch+0x216/0x900
> [ 261.106048] ? rcu_do_batch+0x216/0x900
> [ 261.106052] inet_release+0x42/0x80
> [ 261.106058] sock_release+0x20/0x70
> [ 261.106063] mptcp_pm_addr_entry_free+0x3b/0x60
> [ 261.106067] rcu_do_batch+0x289/0x900
> [ 261.106078] rcu_core+0x27d/0x450
> [ 261.106085] __do_softirq+0xd5/0x485
> [ 261.106096] asm_call_irq_on_stack+0xf/0x20
> [ 261.106098] </IRQ>
> [ 261.106103] do_softirq_own_stack+0x5b/0x70
> [ 261.106106] __irq_exit_rcu+0xda/0x120
> [ 261.106110] irq_exit_rcu+0xa/0x20
> [ 261.106113] sysvec_apic_timer_interrupt+0x4b/0xa0
> [ 261.106118] asm_sysvec_apic_timer_interrupt+0x12/0x20
> [ 261.106122] RIP: 0010:cpuidle_enter_state+0xfa/0x470
>
Oops, my mistake. Yeah, I can see how sock_release() is not able to run in
a regular rcu handler.
Instead of call_rcu(), try queue_rcu_work(). That will run the handler in
a context where it can sleep, after the rcu grace period.
Look at nfc_genl_rcv_nl_event() and nfc_urelease_event_work(), which use a
dynamically allocated work struct. Use 'struct rcu_work' instead of
'struct work_struct' and queue_rcu_work(system_wq, rwork) instead of
schedule_work().
Mat
>
>>>
>>> return ret;
>>> @@ -934,6 +996,8 @@ static void __flush_addrs(struct net *net, struct list_head *list)
>>> cur = list_entry(list->next,
>>> struct mptcp_pm_addr_entry, list);
>>> mptcp_nl_remove_subflow_and_signal_addr(net, &cur->addr);
>>> + if (cur->lsk)
>>> + sock_release(cur->lsk);
>>> list_del_rcu(&cur->list);
>>> kfree_rcu(cur, rcu);
>>
>> Same issue as above with sock_release() and rcu.
>>
>>
>> Mat
>>
>>> }
>>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>>> index 8ec9e4582d18..79e1b34ecb53 100644
>>> --- a/net/mptcp/protocol.c
>>> +++ b/net/mptcp/protocol.c
>>> @@ -49,7 +49,7 @@ static void __mptcp_check_send_data_fin(struct sock *sk);
>>> * completed yet or has failed, return the subflow socket.
>>> * Otherwise return NULL.
>>> */
>>> -static struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk)
>>> +struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk)
>>> {
>>> if (!msk->subflow || READ_ONCE(msk->can_ack))
>>> return NULL;
>>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>>> index d6400ad2d615..a2a031cca97a 100644
>>> --- a/net/mptcp/protocol.h
>>> +++ b/net/mptcp/protocol.h
>>> @@ -473,11 +473,14 @@ void mptcp_subflow_shutdown(struct sock *sk, struct sock *ssk, int how);
>>> void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
>>> struct mptcp_subflow_context *subflow);
>>> void mptcp_subflow_reset(struct sock *ssk);
>>> +struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk);
>>>
>>> /* called with sk socket lock held */
>>> int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
>>> const struct mptcp_addr_info *remote);
>>> int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock);
>>> +void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
>>> + struct sockaddr_storage *addr);
>>>
>>> static inline void mptcp_subflow_tcp_fallback(struct sock *sk,
>>> struct mptcp_subflow_context *ctx)
>>> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
>>> index 73e66a406d99..c64a1c41a29b 100644
>>> --- a/net/mptcp/subflow.c
>>> +++ b/net/mptcp/subflow.c
>>> @@ -1073,8 +1073,8 @@ void mptcpv6_handle_mapped(struct sock *sk, bool mapped)
>>> }
>>> #endif
>>>
>>> -static void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
>>> - struct sockaddr_storage *addr)
>>> +void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
>>> + struct sockaddr_storage *addr)
>>> {
>>> memset(addr, 0, sizeof(*addr));
>>> addr->ss_family = info->family;
>>> --
>>> 2.29.2
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 1/8] mptcp: create the listening socket for new port
@ 2020-12-18 23:25 Geliang Tang
0 siblings, 0 replies; 4+ messages in thread
From: Geliang Tang @ 2020-12-18 23:25 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 10719 bytes --]
Hi Mat,
Mat Martineau <mathew.j.martineau(a)linux.intel.com> 于2020年12月18日周五 上午8:14写道：
>
> On Sun, 13 Dec 2020, Geliang Tang wrote:
>
> > This patch created a listening socket when an address with a port-number
> > is added by PM netlink. Then binded the new port to the socket, and
> > listened for the connection.
> >
> > Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
> > ---
> > net/mptcp/pm_netlink.c | 64 ++++++++++++++++++++++++++++++++++++++++++
> > net/mptcp/protocol.c | 2 +-
> > net/mptcp/protocol.h | 3 ++
> > net/mptcp/subflow.c | 4 +--
> > 4 files changed, 70 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > index 9b1f6298bbdb..1548efb22a1b 100644
> > --- a/net/mptcp/pm_netlink.c
> > +++ b/net/mptcp/pm_netlink.c
> > @@ -26,6 +26,7 @@ struct mptcp_pm_addr_entry {
> > struct list_head list;
> > struct mptcp_addr_info addr;
> > struct rcu_head rcu;
> > + struct socket *lsk;
> > };
> >
> > struct mptcp_pm_add_entry {
> > @@ -613,6 +614,53 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
> > return ret;
> > }
> >
> > +static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
> > + struct mptcp_pm_addr_entry *entry)
> > +{
> > + struct sockaddr_storage addr;
> > + struct mptcp_sock *msk;
> > + struct socket *ssock;
> > + int backlog = 1024;
> > + int err;
> > +
> > + err = sock_create_kern(sock_net(sk), entry->addr.family,
> > + SOCK_STREAM, IPPROTO_MPTCP, &entry->lsk);
> > + if (err)
> > + return err;
> > +
> > + msk = mptcp_sk(entry->lsk->sk);
> > + if (!msk) {
> > + err = -EINVAL;
> > + goto out;
> > + }
> > +
> > + ssock = __mptcp_nmpc_socket(msk);
> > + if (!ssock) {
> > + err = -EINVAL;
> > + goto out;
> > + }
> > +
> > + mptcp_info2sockaddr(&entry->addr, &addr);
> > + err = kernel_bind(ssock, (struct sockaddr *)&addr,
> > + sizeof(struct sockaddr_in));
> > + if (err) {
> > + pr_warn("kernel_bind error, err=%d", err);
> > + goto out;
> > + }
> > +
> > + err = kernel_listen(ssock, backlog);
> > + if (err) {
> > + pr_warn("kernel_listen error, err=%d", err);
> > + goto out;
> > + }
> > +
> > + return 0;
> > +
> > +out:
> > + sock_release(entry->lsk);
> > + return err;
> > +}
> > +
> > int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
> > {
> > struct mptcp_pm_addr_entry *entry;
> > @@ -657,6 +705,8 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
> > entry->addr.ifindex = 0;
> > entry->addr.flags = 0;
> > entry->addr.id = 0;
> > + entry->addr.port = 0;
> > + entry->lsk = NULL;
> > ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
> > if (ret < 0)
> > kfree(entry);
> > @@ -808,9 +858,19 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
> > }
> >
> > *entry = addr;
> > + if (entry->addr.port) {
> > + ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
> > + if (ret) {
> > + GENL_SET_ERR_MSG(info, "create listen socket error");
> > + kfree(entry);
> > + return ret;
> > + }
> > + }
> > ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
> > if (ret < 0) {
> > GENL_SET_ERR_MSG(info, "too many addresses or duplicate one");
> > + if (entry->lsk)
> > + sock_release(entry->lsk);
> > kfree(entry);
> > return ret;
> > }
> > @@ -921,6 +981,8 @@ static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info)
> > spin_unlock_bh(&pernet->lock);
> >
> > mptcp_nl_remove_subflow_and_signal_addr(sock_net(skb->sk), &entry->addr);
> > + if (entry->lsk)
> > + sock_release(entry->lsk);
> > kfree_rcu(entry, rcu);
>
> Releasing the socket here could be racy - since the list is rcu-protected,
> the entry could still be accessed (which is why it's freed with
> kfree_rcu()). Rather than calling kfree_rcu(), use a custom callback with
> call_rcu() that will both release lsk and kfree the list entry.
>
It dosen't work. I fixed it like this:
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 5f80b886aecb..d6b937bffb43 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1010,6 +1010,16 @@ static int
mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
return 0;
}
+static void mptcp_pm_addr_entry_free(struct rcu_head *head)
+{
+ struct mptcp_pm_addr_entry *entry;
+
+ entry = container_of(head, struct mptcp_pm_addr_entry, rcu);
+ if (entry->lsk)
+ sock_release(entry->lsk);
+ kfree(entry);
+}
+
static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info)
{
struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
@@ -1039,9 +1049,7 @@ static int mptcp_nl_cmd_del_addr(struct sk_buff
*skb, struct genl_info *info)
spin_unlock_bh(&pernet->lock);
mptcp_nl_remove_subflow_and_signal_addr(sock_net(skb->sk), &entry->addr);
- if (entry->lsk)
- sock_release(entry->lsk);
- kfree_rcu(entry, rcu);
+ call_rcu(&entry->rcu, mptcp_pm_addr_entry_free);
return ret;
}
@@ -1054,10 +1062,8 @@ static void __flush_addrs(struct net *net,
struct list_head *list)
cur = list_entry(list->next,
struct mptcp_pm_addr_entry, list);
mptcp_nl_remove_subflow_and_signal_addr(net, &cur->addr);
- if (cur->lsk)
- sock_release(cur->lsk);
list_del_rcu(&cur->list);
- kfree_rcu(cur, rcu);
+ call_rcu(&cur->rcu, mptcp_pm_addr_entry_free);
}
}
--
2.29.2
But I got this error:
[ 261.105979] MPTCP: sock_release 0000000058a9d993
[ 261.105985] BUG: sleeping function called from invalid context at
net/core/sock.c:3048
[ 261.105987] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid:
0, name: swapper/2
[ 261.105990] 1 lock held by swapper/2/0:
[ 261.105992] #0: ffffffffac5a68e0 (rcu_callback){....}-{0:0}, at:
rcu_do_batch+0x216/0x900
[ 261.106009] CPU: 2 PID: 0 Comm: swapper/2 Kdump: loaded Not tainted
5.10.0-mptcp+ #429
[ 261.106011] Hardware name: TIMI Mi Laptop Pro 15/TM1905, BIOS
XMACM500P0301 04/08/2020
[ 261.106013] Call Trace:
[ 261.106016] <IRQ>
[ 261.106022] dump_stack+0x8b/0xb0
[ 261.106028] ___might_sleep.cold+0xb6/0xc6
[ 261.106033] lock_sock_nested+0x28/0x90
[ 261.106040] mptcp_close+0x20/0x2f0
[ 261.106043] ? rcu_do_batch+0x216/0x900
[ 261.106048] ? rcu_do_batch+0x216/0x900
[ 261.106052] inet_release+0x42/0x80
[ 261.106058] sock_release+0x20/0x70
[ 261.106063] mptcp_pm_addr_entry_free+0x3b/0x60
[ 261.106067] rcu_do_batch+0x289/0x900
[ 261.106078] rcu_core+0x27d/0x450
[ 261.106085] __do_softirq+0xd5/0x485
[ 261.106096] asm_call_irq_on_stack+0xf/0x20
[ 261.106098] </IRQ>
[ 261.106103] do_softirq_own_stack+0x5b/0x70
[ 261.106106] __irq_exit_rcu+0xda/0x120
[ 261.106110] irq_exit_rcu+0xa/0x20
[ 261.106113] sysvec_apic_timer_interrupt+0x4b/0xa0
[ 261.106118] asm_sysvec_apic_timer_interrupt+0x12/0x20
[ 261.106122] RIP: 0010:cpuidle_enter_state+0xfa/0x470
-Geliang
> >
> > return ret;
> > @@ -934,6 +996,8 @@ static void __flush_addrs(struct net *net, struct list_head *list)
> > cur = list_entry(list->next,
> > struct mptcp_pm_addr_entry, list);
> > mptcp_nl_remove_subflow_and_signal_addr(net, &cur->addr);
> > + if (cur->lsk)
> > + sock_release(cur->lsk);
> > list_del_rcu(&cur->list);
> > kfree_rcu(cur, rcu);
>
> Same issue as above with sock_release() and rcu.
>
>
> Mat
>
> > }
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index 8ec9e4582d18..79e1b34ecb53 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -49,7 +49,7 @@ static void __mptcp_check_send_data_fin(struct sock *sk);
> > * completed yet or has failed, return the subflow socket.
> > * Otherwise return NULL.
> > */
> > -static struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk)
> > +struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk)
> > {
> > if (!msk->subflow || READ_ONCE(msk->can_ack))
> > return NULL;
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index d6400ad2d615..a2a031cca97a 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -473,11 +473,14 @@ void mptcp_subflow_shutdown(struct sock *sk, struct sock *ssk, int how);
> > void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
> > struct mptcp_subflow_context *subflow);
> > void mptcp_subflow_reset(struct sock *ssk);
> > +struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk);
> >
> > /* called with sk socket lock held */
> > int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
> > const struct mptcp_addr_info *remote);
> > int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock);
> > +void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
> > + struct sockaddr_storage *addr);
> >
> > static inline void mptcp_subflow_tcp_fallback(struct sock *sk,
> > struct mptcp_subflow_context *ctx)
> > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> > index 73e66a406d99..c64a1c41a29b 100644
> > --- a/net/mptcp/subflow.c
> > +++ b/net/mptcp/subflow.c
> > @@ -1073,8 +1073,8 @@ void mptcpv6_handle_mapped(struct sock *sk, bool mapped)
> > }
> > #endif
> >
> > -static void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
> > - struct sockaddr_storage *addr)
> > +void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
> > + struct sockaddr_storage *addr)
> > {
> > memset(addr, 0, sizeof(*addr));
> > addr->ss_family = info->family;
> > --
> > 2.29.2
>
> --
> Mat Martineau
> Intel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 1/8] mptcp: create the listening socket for new port
@ 2020-12-18 0:14 Mat Martineau
0 siblings, 0 replies; 4+ messages in thread
From: Mat Martineau @ 2020-12-18 0:14 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 6279 bytes --]
On Sun, 13 Dec 2020, Geliang Tang wrote:
> This patch created a listening socket when an address with a port-number
> is added by PM netlink. Then binded the new port to the socket, and
> listened for the connection.
>
> Signed-off-by: Geliang Tang <geliangtang(a)gmail.com>
> ---
> net/mptcp/pm_netlink.c | 64 ++++++++++++++++++++++++++++++++++++++++++
> net/mptcp/protocol.c | 2 +-
> net/mptcp/protocol.h | 3 ++
> net/mptcp/subflow.c | 4 +--
> 4 files changed, 70 insertions(+), 3 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 9b1f6298bbdb..1548efb22a1b 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -26,6 +26,7 @@ struct mptcp_pm_addr_entry {
> struct list_head list;
> struct mptcp_addr_info addr;
> struct rcu_head rcu;
> + struct socket *lsk;
> };
>
> struct mptcp_pm_add_entry {
> @@ -613,6 +614,53 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
> return ret;
> }
>
> +static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
> + struct mptcp_pm_addr_entry *entry)
> +{
> + struct sockaddr_storage addr;
> + struct mptcp_sock *msk;
> + struct socket *ssock;
> + int backlog = 1024;
> + int err;
> +
> + err = sock_create_kern(sock_net(sk), entry->addr.family,
> + SOCK_STREAM, IPPROTO_MPTCP, &entry->lsk);
> + if (err)
> + return err;
> +
> + msk = mptcp_sk(entry->lsk->sk);
> + if (!msk) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + ssock = __mptcp_nmpc_socket(msk);
> + if (!ssock) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + mptcp_info2sockaddr(&entry->addr, &addr);
> + err = kernel_bind(ssock, (struct sockaddr *)&addr,
> + sizeof(struct sockaddr_in));
> + if (err) {
> + pr_warn("kernel_bind error, err=%d", err);
> + goto out;
> + }
> +
> + err = kernel_listen(ssock, backlog);
> + if (err) {
> + pr_warn("kernel_listen error, err=%d", err);
> + goto out;
> + }
> +
> + return 0;
> +
> +out:
> + sock_release(entry->lsk);
> + return err;
> +}
> +
> int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
> {
> struct mptcp_pm_addr_entry *entry;
> @@ -657,6 +705,8 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
> entry->addr.ifindex = 0;
> entry->addr.flags = 0;
> entry->addr.id = 0;
> + entry->addr.port = 0;
> + entry->lsk = NULL;
> ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
> if (ret < 0)
> kfree(entry);
> @@ -808,9 +858,19 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
> }
>
> *entry = addr;
> + if (entry->addr.port) {
> + ret = mptcp_pm_nl_create_listen_socket(skb->sk, entry);
> + if (ret) {
> + GENL_SET_ERR_MSG(info, "create listen socket error");
> + kfree(entry);
> + return ret;
> + }
> + }
> ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
> if (ret < 0) {
> GENL_SET_ERR_MSG(info, "too many addresses or duplicate one");
> + if (entry->lsk)
> + sock_release(entry->lsk);
> kfree(entry);
> return ret;
> }
> @@ -921,6 +981,8 @@ static int mptcp_nl_cmd_del_addr(struct sk_buff *skb, struct genl_info *info)
> spin_unlock_bh(&pernet->lock);
>
> mptcp_nl_remove_subflow_and_signal_addr(sock_net(skb->sk), &entry->addr);
> + if (entry->lsk)
> + sock_release(entry->lsk);
> kfree_rcu(entry, rcu);
Releasing the socket here could be racy - since the list is rcu-protected,
the entry could still be accessed (which is why it's freed with
kfree_rcu()). Rather than calling kfree_rcu(), use a custom callback with
call_rcu() that will both release lsk and kfree the list entry.
>
> return ret;
> @@ -934,6 +996,8 @@ static void __flush_addrs(struct net *net, struct list_head *list)
> cur = list_entry(list->next,
> struct mptcp_pm_addr_entry, list);
> mptcp_nl_remove_subflow_and_signal_addr(net, &cur->addr);
> + if (cur->lsk)
> + sock_release(cur->lsk);
> list_del_rcu(&cur->list);
> kfree_rcu(cur, rcu);
Same issue as above with sock_release() and rcu.
Mat
> }
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 8ec9e4582d18..79e1b34ecb53 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -49,7 +49,7 @@ static void __mptcp_check_send_data_fin(struct sock *sk);
> * completed yet or has failed, return the subflow socket.
> * Otherwise return NULL.
> */
> -static struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk)
> +struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk)
> {
> if (!msk->subflow || READ_ONCE(msk->can_ack))
> return NULL;
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index d6400ad2d615..a2a031cca97a 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -473,11 +473,14 @@ void mptcp_subflow_shutdown(struct sock *sk, struct sock *ssk, int how);
> void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
> struct mptcp_subflow_context *subflow);
> void mptcp_subflow_reset(struct sock *ssk);
> +struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk);
>
> /* called with sk socket lock held */
> int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
> const struct mptcp_addr_info *remote);
> int mptcp_subflow_create_socket(struct sock *sk, struct socket **new_sock);
> +void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
> + struct sockaddr_storage *addr);
>
> static inline void mptcp_subflow_tcp_fallback(struct sock *sk,
> struct mptcp_subflow_context *ctx)
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 73e66a406d99..c64a1c41a29b 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1073,8 +1073,8 @@ void mptcpv6_handle_mapped(struct sock *sk, bool mapped)
> }
> #endif
>
> -static void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
> - struct sockaddr_storage *addr)
> +void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
> + struct sockaddr_storage *addr)
> {
> memset(addr, 0, sizeof(*addr));
> addr->ss_family = info->family;
> --
> 2.29.2
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 1/8] mptcp: create the listening socket for new port
@ 2020-12-15 5:49 Geliang Tang
0 siblings, 0 replies; 4+ messages in thread
From: Geliang Tang @ 2020-12-15 5:49 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 611 bytes --]
Geliang Tang <geliangtang(a)gmail.com> 于2020年12月13日周日 上午9:52写道：
>
> This patch created a listening socket when an address with a port-number
> is added by PM netlink. Then binded the new port to the socket, and
> listened for the connection.
Please update the commit message:
---
This patch created a listening socket when an address with a port-number
is added by PM netlink. Then binded the new port to the socket, and
listened for the connection.
When the address is removed or the addresses are flushed by PM netlink,
release the listening socket.
---
-Geliang
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-19 0:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-19 0:23 [MPTCP] Re: [MPTCP][PATCH v8 mptcp-next 1/8] mptcp: create the listening socket for new port Mat Martineau
-- strict thread matches above, loose matches on Subject: below --
2020-12-18 23:25 Geliang Tang
2020-12-18 0:14 Mat Martineau
2020-12-15 5:49 Geliang Tang
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.