| Message ID | 20210615232707.835258-1-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d917c35a451e4ebba5c12a51c92cbddce958c91e |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] net: qualcomm: rmnet: Allow partial updates of IFLA_FLAGS | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 5 of 5 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 18 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
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; >
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>
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
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;
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(-)