From: Geliang Tang <tanggeliang@kylinos.cn>

Just the same as userspace PM, a new parameter needs_id is added for
in-kernel PM mptcp_pm_nl_append_new_local_addr() too.

Add a new helper mptcp_pm_has_addr_attr_id() to check whether an address
ID is set from PM or not.

In mptcp_pm_nl_get_local_id(), needs_id is always true, but in
mptcp_pm_nl_add_addr_doit(), pass mptcp_pm_has_addr_attr_id() to
needs_it.

Fixes: efd5a4c04e18 ("mptcp: add the address ID assignment bitmap")
Cc: stable@vger.kernel.org
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Reviewed-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 584f3894262634596532cf43a5e782e34a0ce374)
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Notes:
 - conflicts in pm_netlink.c because the new helper function expected to
   be on top of mptcp_pm_nl_add_addr_doit() which has been recently
   renamed in commit 1e07938e29c5 ("net: mptcp: rename netlink handlers
   to mptcp_pm_nl_<blah>_{doit,dumpit}").
 - use mptcp_pm_addr_policy instead of mptcp_pm_address_nl_policy, the
   new name after commit 1d0507f46843 ("net: mptcp: convert netlink from
   small_ops to ops").
---
 net/mptcp/pm_netlink.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index e504a1649da0..4dd47a1fb9aa 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -904,7 +904,8 @@ static void __mptcp_pm_release_addr_entry(struct mptcp_pm_addr_entry *entry)
 }
 
 static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
-					     struct mptcp_pm_addr_entry *entry)
+					     struct mptcp_pm_addr_entry *entry,
+					     bool needs_id)
 {
 	struct mptcp_pm_addr_entry *cur, *del_entry = NULL;
 	unsigned int addr_max;
@@ -952,7 +953,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 		}
 	}
 
-	if (!entry->addr.id) {
+	if (!entry->addr.id && needs_id) {
 find_next:
 		entry->addr.id = find_next_zero_bit(pernet->id_bitmap,
 						    MPTCP_PM_MAX_ADDR_ID + 1,
@@ -963,7 +964,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 		}
 	}
 
-	if (!entry->addr.id)
+	if (!entry->addr.id && needs_id)
 		goto out;
 
 	__set_bit(entry->addr.id, pernet->id_bitmap);
@@ -1095,7 +1096,7 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc
 	entry->ifindex = 0;
 	entry->flags = MPTCP_PM_ADDR_FLAG_IMPLICIT;
 	entry->lsk = NULL;
-	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
+	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry, true);
 	if (ret < 0)
 		kfree(entry);
 
@@ -1311,6 +1312,18 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net)
 	return 0;
 }
 
+static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr,
+				      struct genl_info *info)
+{
+	struct nlattr *tb[MPTCP_PM_ADDR_ATTR_MAX + 1];
+
+	if (!nla_parse_nested_deprecated(tb, MPTCP_PM_ADDR_ATTR_MAX, attr,
+					 mptcp_pm_addr_policy, info->extack) &&
+	    tb[MPTCP_PM_ADDR_ATTR_ID])
+		return true;
+	return false;
+}
+
 static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
 {
 	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
@@ -1352,7 +1365,8 @@ static int mptcp_nl_cmd_add_addr(struct sk_buff *skb, struct genl_info *info)
 			goto out_free;
 		}
 	}
-	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
+	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry,
+						!mptcp_pm_has_addr_attr_id(attr, info));
 	if (ret < 0) {
 		GENL_SET_ERR_MSG_FMT(info, "too many addresses or duplicate one: %d", ret);
 		goto out_free;
-- 
2.43.0

On Mon, Feb 26, 2024 at 10:56:21PM +0100, Matthieu Baerts (NGI0) wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Just the same as userspace PM, a new parameter needs_id is added for
> in-kernel PM mptcp_pm_nl_append_new_local_addr() too.
> 
> Add a new helper mptcp_pm_has_addr_attr_id() to check whether an address
> ID is set from PM or not.
> 
> In mptcp_pm_nl_get_local_id(), needs_id is always true, but in
> mptcp_pm_nl_add_addr_doit(), pass mptcp_pm_has_addr_attr_id() to
> needs_it.
> 
> Fixes: efd5a4c04e18 ("mptcp: add the address ID assignment bitmap")
> Cc: stable@vger.kernel.org
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> Reviewed-by: Mat Martineau <martineau@kernel.org>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit 584f3894262634596532cf43a5e782e34a0ce374)
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Notes:
>  - conflicts in pm_netlink.c because the new helper function expected to
>    be on top of mptcp_pm_nl_add_addr_doit() which has been recently
>    renamed in commit 1e07938e29c5 ("net: mptcp: rename netlink handlers
>    to mptcp_pm_nl_<blah>_{doit,dumpit}").
>  - use mptcp_pm_addr_policy instead of mptcp_pm_address_nl_policy, the
>    new name after commit 1d0507f46843 ("net: mptcp: convert netlink from
>    small_ops to ops").
> ---
>  net/mptcp/pm_netlink.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)

Don't we also need a 5.15.y version of this commit?

All of the backports you sent are now queued up, thanks!

greg k-h

Hi Greg,

On 27/02/2024 11:22, Greg KH wrote:
> On Mon, Feb 26, 2024 at 10:56:21PM +0100, Matthieu Baerts (NGI0) wrote:
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> Just the same as userspace PM, a new parameter needs_id is added for
>> in-kernel PM mptcp_pm_nl_append_new_local_addr() too.
>>
>> Add a new helper mptcp_pm_has_addr_attr_id() to check whether an address
>> ID is set from PM or not.
>>
>> In mptcp_pm_nl_get_local_id(), needs_id is always true, but in
>> mptcp_pm_nl_add_addr_doit(), pass mptcp_pm_has_addr_attr_id() to
>> needs_it.
>>
>> Fixes: efd5a4c04e18 ("mptcp: add the address ID assignment bitmap")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>> Reviewed-by: Mat Martineau <martineau@kernel.org>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>> (cherry picked from commit 584f3894262634596532cf43a5e782e34a0ce374)
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>> Notes:
>>  - conflicts in pm_netlink.c because the new helper function expected to
>>    be on top of mptcp_pm_nl_add_addr_doit() which has been recently
>>    renamed in commit 1e07938e29c5 ("net: mptcp: rename netlink handlers
>>    to mptcp_pm_nl_<blah>_{doit,dumpit}").
>>  - use mptcp_pm_addr_policy instead of mptcp_pm_address_nl_policy, the
>>    new name after commit 1d0507f46843 ("net: mptcp: convert netlink from
>>    small_ops to ops").
>> ---
>>  net/mptcp/pm_netlink.c | 24 +++++++++++++++++++-----
>>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> Don't we also need a 5.15.y version of this commit?

Good point, yes, according to the 'Fixes' tag, we need it as well for
5.15.y.

It looks like no "FAILED: patch" notification has been sent for this
patch for the 5.15-stable tree. Is it normal?

I'm asking this because I rely on these notifications to know if I need
to help to fix conflicts. I don't regularly track if patches we sent
upstream with 'Cc: stable' & 'Fixes' tags have been backported. It is
just to know if we need to modify our way of working :)

> All of the backports you sent are now queued up, thanks!

Thank you for all the great work!

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

On Tue, Feb 27, 2024 at 12:04:22PM +0100, Matthieu Baerts wrote:
> Hi Greg,
> 
> On 27/02/2024 11:22, Greg KH wrote:
> > On Mon, Feb 26, 2024 at 10:56:21PM +0100, Matthieu Baerts (NGI0) wrote:
> >> From: Geliang Tang <tanggeliang@kylinos.cn>
> >>
> >> Just the same as userspace PM, a new parameter needs_id is added for
> >> in-kernel PM mptcp_pm_nl_append_new_local_addr() too.
> >>
> >> Add a new helper mptcp_pm_has_addr_attr_id() to check whether an address
> >> ID is set from PM or not.
> >>
> >> In mptcp_pm_nl_get_local_id(), needs_id is always true, but in
> >> mptcp_pm_nl_add_addr_doit(), pass mptcp_pm_has_addr_attr_id() to
> >> needs_it.
> >>
> >> Fixes: efd5a4c04e18 ("mptcp: add the address ID assignment bitmap")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> >> Reviewed-by: Mat Martineau <martineau@kernel.org>
> >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> >> Signed-off-by: David S. Miller <davem@davemloft.net>
> >> (cherry picked from commit 584f3894262634596532cf43a5e782e34a0ce374)
> >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> >> ---
> >> Notes:
> >>  - conflicts in pm_netlink.c because the new helper function expected to
> >>    be on top of mptcp_pm_nl_add_addr_doit() which has been recently
> >>    renamed in commit 1e07938e29c5 ("net: mptcp: rename netlink handlers
> >>    to mptcp_pm_nl_<blah>_{doit,dumpit}").
> >>  - use mptcp_pm_addr_policy instead of mptcp_pm_address_nl_policy, the
> >>    new name after commit 1d0507f46843 ("net: mptcp: convert netlink from
> >>    small_ops to ops").
> >> ---
> >>  net/mptcp/pm_netlink.c | 24 +++++++++++++++++++-----
> >>  1 file changed, 19 insertions(+), 5 deletions(-)
> > 
> > Don't we also need a 5.15.y version of this commit?
> 
> Good point, yes, according to the 'Fixes' tag, we need it as well for
> 5.15.y.
> 
> It looks like no "FAILED: patch" notification has been sent for this
> patch for the 5.15-stable tree. Is it normal?

Hm, odd, I don't know why I didn't send that out, that's a fault on my
side, sorry about that.

So yes, we do need this, I've just now sent the email if you trigger off
of that :)

> I'm asking this because I rely on these notifications to know if I need
> to help to fix conflicts. I don't regularly track if patches we sent
> upstream with 'Cc: stable' & 'Fixes' tags have been backported. It is
> just to know if we need to modify our way of working :)

No, your way of working is WONDERFUL from my side at least, I have no
complaints at all!

thanks,

greg k-h

On 27/02/2024 14:01, Greg KH wrote:
> On Tue, Feb 27, 2024 at 12:04:22PM +0100, Matthieu Baerts wrote:
>> Hi Greg,
>>
>> On 27/02/2024 11:22, Greg KH wrote:
>>> On Mon, Feb 26, 2024 at 10:56:21PM +0100, Matthieu Baerts (NGI0) wrote:
>>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>>
>>>> Just the same as userspace PM, a new parameter needs_id is added for
>>>> in-kernel PM mptcp_pm_nl_append_new_local_addr() too.
>>>>
>>>> Add a new helper mptcp_pm_has_addr_attr_id() to check whether an address
>>>> ID is set from PM or not.
>>>>
>>>> In mptcp_pm_nl_get_local_id(), needs_id is always true, but in
>>>> mptcp_pm_nl_add_addr_doit(), pass mptcp_pm_has_addr_attr_id() to
>>>> needs_it.
>>>>
>>>> Fixes: efd5a4c04e18 ("mptcp: add the address ID assignment bitmap")
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
>>>> Reviewed-by: Mat Martineau <martineau@kernel.org>
>>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>>> (cherry picked from commit 584f3894262634596532cf43a5e782e34a0ce374)
>>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>>> ---
>>>> Notes:
>>>>  - conflicts in pm_netlink.c because the new helper function expected to
>>>>    be on top of mptcp_pm_nl_add_addr_doit() which has been recently
>>>>    renamed in commit 1e07938e29c5 ("net: mptcp: rename netlink handlers
>>>>    to mptcp_pm_nl_<blah>_{doit,dumpit}").
>>>>  - use mptcp_pm_addr_policy instead of mptcp_pm_address_nl_policy, the
>>>>    new name after commit 1d0507f46843 ("net: mptcp: convert netlink from
>>>>    small_ops to ops").
>>>> ---
>>>>  net/mptcp/pm_netlink.c | 24 +++++++++++++++++++-----
>>>>  1 file changed, 19 insertions(+), 5 deletions(-)
>>>
>>> Don't we also need a 5.15.y version of this commit?
>>
>> Good point, yes, according to the 'Fixes' tag, we need it as well for
>> 5.15.y.
>>
>> It looks like no "FAILED: patch" notification has been sent for this
>> patch for the 5.15-stable tree. Is it normal?
> 
> Hm, odd, I don't know why I didn't send that out, that's a fault on my
> side, sorry about that.
> 
> So yes, we do need this, I've just now sent the email if you trigger off
> of that :)

All good, it happens! :)

I will check that one later. I'm currently tracking one (or two?)
regression(s) in v6.1. It looks like they were already present in the
last v6.1 tag (v6.1.79). Patches will follow.

>> I'm asking this because I rely on these notifications to know if I need
>> to help to fix conflicts. I don't regularly track if patches we sent
>> upstream with 'Cc: stable' & 'Fixes' tags have been backported. It is
>> just to know if we need to modify our way of working :)
> 
> No, your way of working is WONDERFUL from my side at least, I have no
> complaints at all!

Great! We will then continue to track 'FAILED: patch' notifications, and
rely on your excellent work selecting all patches that need to be
backported.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.