diff mbox series

[mptcp-next,v1,6/6] mptcp: pm: in-kernel: drop changed parameter of set_flags

Message ID 1e2f2a9bcdf69bd241dec4c5455c4aa135927cb6.1740638334.git.tanggeliang@kylinos.cn (mailing list archive)
State Superseded, archived
Delegated to: Matthieu Baerts
Headers show
Series BPF path manager, part 5 | expand

Checks

Context Check Description
matttbe/checkpatch success total: 0 errors, 0 warnings, 0 checks, 31 lines checked
matttbe/shellcheck success MPTCP selftests files have not been modified
matttbe/build warning Build error with: make W=1 net/mptcp/pm_netlink.o
matttbe/KVM_Validation__normal success Success! ✅
matttbe/KVM_Validation__debug success Success! ✅
matttbe/KVM_Validation__btf-normal__only_bpftest_all_ success Success! ✅
matttbe/KVM_Validation__btf-debug__only_bpftest_all_ success Success! ✅

Commit Message

Geliang Tang Feb. 27, 2025, 6:43 a.m. UTC 
From: Geliang Tang <tanggeliang@kylinos.cn>

To drop the additional "changed" parameter of mptcp_nl_set_flags(),
store "entry->flags" to "remote->flags" before modifying it in
mptcp_pm_nl_set_flags(), so that "changed" value can be obtained by
comparing "local->flags" and "remote->flags" in mptcp_nl_set_flags().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_netlink.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Matthieu Baerts Feb. 27, 2025, 12:30 p.m. UTC | #1 
Hi Geliang,

On 27/02/2025 07:43, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> To drop the additional "changed" parameter of mptcp_nl_set_flags(),
> store "entry->flags" to "remote->flags" before modifying it in
> mptcp_pm_nl_set_flags(), so that "changed" value can be obtained by
> comparing "local->flags" and "remote->flags" in mptcp_nl_set_flags().

Here as well, no need to change the code: it doesn't make sense to deal
with 'remote' as a workaround to have the same interface that is not
needed from what I understood. No?

> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm_netlink.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 053f2bec9042..2f22c3bf88e2 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1916,13 +1916,16 @@ static void mptcp_pm_nl_fullmesh(struct mptcp_sock *msk,
>  
>  static void mptcp_nl_set_flags(struct net *net,
>  			       struct mptcp_pm_addr_entry *local,
> -			       u8 changed)
> +			       struct mptcp_pm_addr_entry *remote)
>  {
>  	u8 is_subflow = !!(local->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW);
>  	u8 bkup = !!(local->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
> +	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
> +			   MPTCP_PM_ADDR_FLAG_FULLMESH;
>  	long s_slot = 0, s_num = 0;
>  	struct mptcp_sock *msk;
>  
> +	changed = (local->flags ^ remote->flags) & mask;
>  	if (changed == MPTCP_PM_ADDR_FLAG_FULLMESH && !is_subflow)
>  		return;
>  
> @@ -1987,12 +1990,13 @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
>  		return -EINVAL;
>  	}
>  
> +	remote->flags = entry->flags;

(I guess you wanted to use "remote->flags = changed" or something like
that, but this code should not be modified I think, it doesn't make
sense to deal with a "remote" here)

>  	changed = (local->flags ^ entry->flags) & mask;
>  	entry->flags = (entry->flags & ~mask) | (local->flags & mask);
>  	*local = *entry;
>  	spin_unlock_bh(&pernet->lock);
>  
> -	mptcp_nl_set_flags(net, local, changed);
> +	mptcp_nl_set_flags(net, local, remote);
>  	return 0;
>  }
>  
Cheers,
Matt
kernel test robot Feb. 28, 2025, 7:56 a.m. UTC | #2 
Hi Geliang,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mptcp/export]
[cannot apply to mptcp/export-net linus/master v6.14-rc4 next-20250227]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Geliang-Tang/mptcp-pm-use-pm-variable-instead-of-msk-pm/20250227-144528
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
patch link:    https://lore.kernel.org/r/1e2f2a9bcdf69bd241dec4c5455c4aa135927cb6.1740638334.git.tanggeliang%40kylinos.cn
patch subject: [PATCH mptcp-next v1 6/6] mptcp: pm: in-kernel: drop changed parameter of set_flags
config: riscv-randconfig-002-20250228 (https://download.01.org/0day-ci/archive/20250228/202502281544.ZIQZO9cy-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250228/202502281544.ZIQZO9cy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502281544.ZIQZO9cy-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/mptcp/pm_netlink.c: In function 'mptcp_pm_nl_set_flags':
>> net/mptcp/pm_netlink.c:1959:12: warning: variable 'changed' set but not used [-Wunused-but-set-variable]
    1959 |         u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
         |            ^~~~~~~


vim +/changed +1959 net/mptcp/pm_netlink.c

0f9f696a502e1b0 Geliang Tang           2021-01-08  1953  
c7f25f7987c060b Geliang Tang           2025-02-07  1954  int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
de258815006a162 Geliang Tang           2025-02-27  1955  			  struct mptcp_pm_addr_entry *remote,
c7f25f7987c060b Geliang Tang           2025-02-07  1956  			  struct genl_info *info)
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1957  {
c7f25f7987c060b Geliang Tang           2025-02-07  1958  	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
6ba7ce89905c5d5 Geliang Tang           2023-06-08 @1959  	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1960  			   MPTCP_PM_ADDR_FLAG_FULLMESH;
2c8971c04f745de Geliang Tang           2025-02-07  1961  	struct net *net = genl_info_net(info);
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1962  	struct mptcp_pm_addr_entry *entry;
6a42477fe4491e4 Geliang Tang           2024-03-05  1963  	struct pm_nl_pernet *pernet;
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1964  	u8 lookup_by_id = 0;
8cdc56f99e6c33a Matthieu Baerts (NGI0  2025-02-07  1965) 
6a42477fe4491e4 Geliang Tang           2024-03-05  1966  	pernet = pm_nl_get_pernet(net);
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1967  
c7f25f7987c060b Geliang Tang           2025-02-07  1968  	if (local->addr.family == AF_UNSPEC) {
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1969  		lookup_by_id = 1;
c7f25f7987c060b Geliang Tang           2025-02-07  1970  		if (!local->addr.id) {
a25a8b10491bb94 Matthieu Baerts (NGI0  2025-02-07  1971) 			NL_SET_ERR_MSG_ATTR(info->extack, attr,
a25a8b10491bb94 Matthieu Baerts (NGI0  2025-02-07  1972) 					    "missing address ID");
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1973  			return -EOPNOTSUPP;
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1974  		}
a4d68b160240815 Geliang Tang           2024-03-05  1975  	}
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1976  
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1977  	spin_lock_bh(&pernet->lock);
c7f25f7987c060b Geliang Tang           2025-02-07  1978  	entry = lookup_by_id ? __lookup_addr_by_id(pernet, local->addr.id) :
c7f25f7987c060b Geliang Tang           2025-02-07  1979  			       __lookup_addr(pernet, &local->addr);
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1980  	if (!entry) {
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1981  		spin_unlock_bh(&pernet->lock);
a25a8b10491bb94 Matthieu Baerts (NGI0  2025-02-07  1982) 		NL_SET_ERR_MSG_ATTR(info->extack, attr, "address not found");
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1983  		return -EINVAL;
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1984  	}
c7f25f7987c060b Geliang Tang           2025-02-07  1985  	if ((local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
1bb0d1348546ad0 Matthieu Baerts (NGI0  2025-01-23  1986) 	    (entry->flags & (MPTCP_PM_ADDR_FLAG_SIGNAL |
1bb0d1348546ad0 Matthieu Baerts (NGI0  2025-01-23  1987) 			     MPTCP_PM_ADDR_FLAG_IMPLICIT))) {
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1988  		spin_unlock_bh(&pernet->lock);
a25a8b10491bb94 Matthieu Baerts (NGI0  2025-02-07  1989) 		NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1990  		return -EINVAL;
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1991  	}
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1992  
c029cf028bb79d2 Geliang Tang           2025-02-27  1993  	remote->flags = entry->flags;
c7f25f7987c060b Geliang Tang           2025-02-07  1994  	changed = (local->flags ^ entry->flags) & mask;
c7f25f7987c060b Geliang Tang           2025-02-07  1995  	entry->flags = (entry->flags & ~mask) | (local->flags & mask);
c7f25f7987c060b Geliang Tang           2025-02-07  1996  	*local = *entry;
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1997  	spin_unlock_bh(&pernet->lock);
6ba7ce89905c5d5 Geliang Tang           2023-06-08  1998  
c029cf028bb79d2 Geliang Tang           2025-02-27  1999  	mptcp_nl_set_flags(net, local, remote);
6ba7ce89905c5d5 Geliang Tang           2023-06-08  2000  	return 0;
6ba7ce89905c5d5 Geliang Tang           2023-06-08  2001  }
6ba7ce89905c5d5 Geliang Tang           2023-06-08  2002
diff mbox series

Patch

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 053f2bec9042..2f22c3bf88e2 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1916,13 +1916,16 @@  static void mptcp_pm_nl_fullmesh(struct mptcp_sock *msk,
 
 static void mptcp_nl_set_flags(struct net *net,
 			       struct mptcp_pm_addr_entry *local,
-			       u8 changed)
+			       struct mptcp_pm_addr_entry *remote)
 {
 	u8 is_subflow = !!(local->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW);
 	u8 bkup = !!(local->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
+	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
+			   MPTCP_PM_ADDR_FLAG_FULLMESH;
 	long s_slot = 0, s_num = 0;
 	struct mptcp_sock *msk;
 
+	changed = (local->flags ^ remote->flags) & mask;
 	if (changed == MPTCP_PM_ADDR_FLAG_FULLMESH && !is_subflow)
 		return;
 
@@ -1987,12 +1990,13 @@  int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
 		return -EINVAL;
 	}
 
+	remote->flags = entry->flags;
 	changed = (local->flags ^ entry->flags) & mask;
 	entry->flags = (entry->flags & ~mask) | (local->flags & mask);
 	*local = *entry;
 	spin_unlock_bh(&pernet->lock);
 
-	mptcp_nl_set_flags(net, local, changed);
+	mptcp_nl_set_flags(net, local, remote);
 	return 0;
 }