linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS
@ 2021-06-15 23:27 Bjorn Andersson
  2021-06-16  1:35 ` Alex Elder
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Bjorn Andersson @ 2021-06-15 23:27 UTC (permalink / raw)
  To: Subash Abhinov Kasiviswanathan, Sean Tranchetti, David S. Miller,
	Jakub Kicinski, Alex Elder
  Cc: netdev, linux-kernel

The idiomatic way to handle the changelink flags/mask pair seems to be
allow partial updates of the driver's link flags. In contrast the rmnet
driver masks the incoming flags and then use that as the new flags.

Change the rmnet driver to follow the common scheme, before the
introduction of IFLA_RMNET_FLAGS handling in iproute2 et al.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Also do the masking dance on newlink, per Subash request
- Add "net-next" to subject prefix

 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 8d51b0cb545c..27b1663c476e 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -163,7 +163,8 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
 		struct ifla_rmnet_flags *flags;
 
 		flags = nla_data(data[IFLA_RMNET_FLAGS]);
-		data_format = flags->flags & flags->mask;
+		data_format &= ~flags->mask;
+		data_format |= flags->flags & flags->mask;
 	}
 
 	netdev_dbg(dev, "data format [0x%08X]\n", data_format);
@@ -336,7 +337,8 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
 
 		old_data_format = port->data_format;
 		flags = nla_data(data[IFLA_RMNET_FLAGS]);
-		port->data_format = flags->flags & flags->mask;
+		port->data_format &= ~flags->mask;
+		port->data_format |= flags->flags & flags->mask;
 
 		if (rmnet_vnd_update_dev_mtu(port, real_dev)) {
 			port->data_format = old_data_format;
-- 
2.31.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS
  2021-06-15 23:27 [PATCH net-next v2] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS Bjorn Andersson
@ 2021-06-16  1:35 ` Alex Elder
  2021-06-16  6:27 ` subashab
  2021-06-16 19:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Alex Elder @ 2021-06-16  1:35 UTC (permalink / raw)
  To: Bjorn Andersson, Subash Abhinov Kasiviswanathan, Sean Tranchetti,
	David S. Miller, Jakub Kicinski
  Cc: netdev, linux-kernel

On 6/15/21 6:27 PM, Bjorn Andersson wrote:
> The idiomatic way to handle the changelink flags/mask pair seems to be
> allow partial updates of the driver's link flags. In contrast the rmnet
> driver masks the incoming flags and then use that as the new flags.
> 
> Change the rmnet driver to follow the common scheme, before the
> introduction of IFLA_RMNET_FLAGS handling in iproute2 et al.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

KS was right, we want the same behavior in both newlink and changelink,
but aside from that, I like this a lot.

It looks good to me.

Reviewed-by: Alex Elder <elder@linaro.org>

> ---
> 
> Changes since v1:
> - Also do the masking dance on newlink, per Subash request
> - Add "net-next" to subject prefix
> 
>  drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> index 8d51b0cb545c..27b1663c476e 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> @@ -163,7 +163,8 @@ static int rmnet_newlink(struct net *src_net, struct net_device *dev,
>  		struct ifla_rmnet_flags *flags;
>  
>  		flags = nla_data(data[IFLA_RMNET_FLAGS]);
> -		data_format = flags->flags & flags->mask;
> +		data_format &= ~flags->mask;
> +		data_format |= flags->flags & flags->mask;
>  	}
>  
>  	netdev_dbg(dev, "data format [0x%08X]\n", data_format);
> @@ -336,7 +337,8 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
>  
>  		old_data_format = port->data_format;
>  		flags = nla_data(data[IFLA_RMNET_FLAGS]);
> -		port->data_format = flags->flags & flags->mask;
> +		port->data_format &= ~flags->mask;
> +		port->data_format |= flags->flags & flags->mask;
>  
>  		if (rmnet_vnd_update_dev_mtu(port, real_dev)) {
>  			port->data_format = old_data_format;
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS
  2021-06-15 23:27 [PATCH net-next v2] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS Bjorn Andersson
  2021-06-16  1:35 ` Alex Elder
@ 2021-06-16  6:27 ` subashab
  2021-06-16 19:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: subashab @ 2021-06-16  6:27 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Sean Tranchetti, David S. Miller, Jakub Kicinski, Alex Elder,
	netdev, linux-kernel

On 2021-06-15 17:27, Bjorn Andersson wrote:
> The idiomatic way to handle the changelink flags/mask pair seems to be
> allow partial updates of the driver's link flags. In contrast the rmnet
> driver masks the incoming flags and then use that as the new flags.
> 
> Change the rmnet driver to follow the common scheme, before the
> introduction of IFLA_RMNET_FLAGS handling in iproute2 et al.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> Changes since v1:
> - Also do the masking dance on newlink, per Subash request
> - Add "net-next" to subject prefix
> 
>  drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> index 8d51b0cb545c..27b1663c476e 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> @@ -163,7 +163,8 @@ static int rmnet_newlink(struct net *src_net,
> struct net_device *dev,
>  		struct ifla_rmnet_flags *flags;
> 
>  		flags = nla_data(data[IFLA_RMNET_FLAGS]);
> -		data_format = flags->flags & flags->mask;
> +		data_format &= ~flags->mask;
> +		data_format |= flags->flags & flags->mask;
>  	}
> 
>  	netdev_dbg(dev, "data format [0x%08X]\n", data_format);
> @@ -336,7 +337,8 @@ static int rmnet_changelink(struct net_device
> *dev, struct nlattr *tb[],
> 
>  		old_data_format = port->data_format;
>  		flags = nla_data(data[IFLA_RMNET_FLAGS]);
> -		port->data_format = flags->flags & flags->mask;
> +		port->data_format &= ~flags->mask;
> +		port->data_format |= flags->flags & flags->mask;
> 
>  		if (rmnet_vnd_update_dev_mtu(port, real_dev)) {
>  			port->data_format = old_data_format;

Reviewed-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS
  2021-06-15 23:27 [PATCH net-next v2] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS Bjorn Andersson
  2021-06-16  1:35 ` Alex Elder
  2021-06-16  6:27 ` subashab
@ 2021-06-16 19:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-06-16 19:20 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: subashab, stranche, davem, kuba, elder, netdev, linux-kernel

Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Tue, 15 Jun 2021 18:27:07 -0500 you wrote:
> The idiomatic way to handle the changelink flags/mask pair seems to be
> allow partial updates of the driver's link flags. In contrast the rmnet
> driver masks the incoming flags and then use that as the new flags.
> 
> Change the rmnet driver to follow the common scheme, before the
> introduction of IFLA_RMNET_FLAGS handling in iproute2 et al.
> 
> [...]

Here is the summary with links:
  - [net-next,v2] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS
    https://git.kernel.org/netdev/net-next/c/d917c35a451e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-16 19:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 23:27 [PATCH net-next v2] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS Bjorn Andersson
2021-06-16  1:35 ` Alex Elder
2021-06-16  6:27 ` subashab
2021-06-16 19:20 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).