| Message ID | 1607017240-10582-1-git-send-email-loic.poulain@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net: rmnet: Adjust virtual device MTU on real device capability | expand |
On 2020-12-03 10:40, Loic Poulain wrote: > A submitted qmap/rmnet packet size can not be larger than the linked > interface (real_dev) MTU. This patch ensures that the rmnet virtual > iface MTU is configured according real device capability. > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > --- > drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c > b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c > index d58b51d..d1d7328 100644 > --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c > +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c > @@ -60,9 +60,14 @@ static netdev_tx_t rmnet_vnd_start_xmit(struct > sk_buff > *skb, > > static int rmnet_vnd_change_mtu(struct net_device *rmnet_dev, int > new_mtu) > { > + struct rmnet_priv *priv = netdev_priv(rmnet_dev); > + > if (new_mtu < 0 || new_mtu > RMNET_MAX_PACKET_SIZE) > return -EINVAL; > > + if (priv->real_dev && (new_mtu + RMNET_NEEDED_HEADROOM) > > priv->real_dev->mtu) > + return -EINVAL; > + > rmnet_dev->mtu = new_mtu; > return 0; > } > @@ -242,6 +247,9 @@ int rmnet_vnd_newlink(u8 id, struct net_device > *rmnet_dev, > > priv->real_dev = real_dev; > > + /* Align default MTU with real_dev MTU */ > + rmnet_vnd_change_mtu(rmnet_dev, real_dev->mtu - > RMNET_NEEDED_HEADROOM); > + > rc = register_netdevice(rmnet_dev); > if (!rc) { > ep->egress_dev = rmnet_dev; This would need similar checks in the NETDEV_PRECHANGEMTU netdev notifier.
Hi Subash, On Fri, 4 Dec 2020 at 22:56, <subashab@codeaurora.org> wrote: > > On 2020-12-03 10:40, Loic Poulain wrote: > > A submitted qmap/rmnet packet size can not be larger than the linked > > interface (real_dev) MTU. This patch ensures that the rmnet virtual > > iface MTU is configured according real device capability. > > > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> [...] > > @@ -242,6 +247,9 @@ int rmnet_vnd_newlink(u8 id, struct net_device > > *rmnet_dev, > > > > priv->real_dev = real_dev; > > > > + /* Align default MTU with real_dev MTU */ > > + rmnet_vnd_change_mtu(rmnet_dev, real_dev->mtu - > > RMNET_NEEDED_HEADROOM); > > + > > rc = register_netdevice(rmnet_dev); > > if (!rc) { > > ep->egress_dev = rmnet_dev; > > This would need similar checks in the NETDEV_PRECHANGEMTU > netdev notifier. What about just returning an error on NETDEV_PRECHANGEMTU notification to prevent real device MTU change while virtual rmnet devices are linked? Not sure there is a more proper and thread safe way to manager that otherwise. Regards, Loic
On Mon, 7 Dec 2020 17:45:17 +0100 Loic Poulain wrote: > > This would need similar checks in the NETDEV_PRECHANGEMTU > > netdev notifier. > > What about just returning an error on NETDEV_PRECHANGEMTU notification > to prevent real device MTU change while virtual rmnet devices are > linked? Not sure there is a more proper and thread safe way to manager > that otherwise. Can't you copy what vlan devices do? That'd seem like a reasonable and well tested precedent, no?
>> What about just returning an error on NETDEV_PRECHANGEMTU notification >> to prevent real device MTU change while virtual rmnet devices are >> linked? Not sure there is a more proper and thread safe way to manager >> that otherwise. > > Can't you copy what vlan devices do? That'd seem like a reasonable and > well tested precedent, no? Could you try this patch. I've tried addressing most of the conditions here. I haven't seen any issues with updating the MTU when rmnet devices are linked. diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c index fcdecdd..8d51b0c 100644 --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c @@ -26,7 +26,7 @@ static int rmnet_is_real_dev_registered(const struct net_device *real_dev) } /* Needs rtnl lock */ -static struct rmnet_port* +struct rmnet_port* rmnet_get_port_rtnl(const struct net_device *real_dev) { return rtnl_dereference(real_dev->rx_handler_data); @@ -253,7 +253,10 @@ static int rmnet_config_notify_cb(struct notifier_block *nb, netdev_dbg(real_dev, "Kernel unregister\n"); rmnet_force_unassociate_device(real_dev); break; - + case NETDEV_CHANGEMTU: + if (rmnet_vnd_validate_real_dev_mtu(real_dev)) + return NOTIFY_BAD; + break; default: break; } @@ -329,9 +332,17 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[], if (data[IFLA_RMNET_FLAGS]) { struct ifla_rmnet_flags *flags; + u32 old_data_format; + old_data_format = port->data_format; flags = nla_data(data[IFLA_RMNET_FLAGS]); port->data_format = flags->flags & flags->mask; + + if (rmnet_vnd_update_dev_mtu(port, real_dev)) { + port->data_format = old_data_format; + NL_SET_ERR_MSG_MOD(extack, "Invalid MTU on real dev"); + return -EINVAL; + } } return 0; diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h index be51598..8d8d469 100644 --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h @@ -73,4 +73,6 @@ int rmnet_add_bridge(struct net_device *rmnet_dev, struct netlink_ext_ack *extack); int rmnet_del_bridge(struct net_device *rmnet_dev, struct net_device *slave_dev); +struct rmnet_port* +rmnet_get_port_rtnl(const struct net_device *real_dev); #endif /* _RMNET_CONFIG_H_ */ diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c index d58b51d..df87883 100644 --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c @@ -58,9 +58,30 @@ static netdev_tx_t rmnet_vnd_start_xmit(struct sk_buff *skb, return NETDEV_TX_OK; } +static int rmnet_vnd_headroom(struct net_device *real_dev) +{ + struct rmnet_port *port; + u32 headroom; + + port = rmnet_get_port_rtnl(real_dev); + + headroom = sizeof(struct rmnet_map_header); + + if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) + headroom += sizeof(struct rmnet_map_dl_csum_trailer); + + return headroom; +} + static int rmnet_vnd_change_mtu(struct net_device *rmnet_dev, int new_mtu) { - if (new_mtu < 0 || new_mtu > RMNET_MAX_PACKET_SIZE) + struct rmnet_priv *priv = netdev_priv(rmnet_dev); + u32 headroom; + + headroom = rmnet_vnd_headroom(priv->real_dev); + + if (new_mtu < 0 || new_mtu > RMNET_MAX_PACKET_SIZE || + new_mtu > (priv->real_dev->mtu - headroom)) return -EINVAL; rmnet_dev->mtu = new_mtu; @@ -229,6 +250,7 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev, { struct rmnet_priv *priv = netdev_priv(rmnet_dev); + u32 headroom; int rc; if (rmnet_get_endpoint(port, id)) { @@ -242,6 +264,13 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev, priv->real_dev = real_dev; + headroom = rmnet_vnd_headroom(real_dev); + + if (rmnet_vnd_change_mtu(rmnet_dev, real_dev->mtu - headroom)) { + NL_SET_ERR_MSG_MOD(extack, "Invalid MTU on real dev"); + return -EINVAL; + } + rc = register_netdevice(rmnet_dev); if (!rc) { ep->egress_dev = rmnet_dev; @@ -283,3 +312,51 @@ int rmnet_vnd_do_flow_control(struct net_device *rmnet_dev, int enable) return 0; } + +int rmnet_vnd_validate_real_dev_mtu(struct net_device *real_dev) +{ + struct hlist_node *tmp_ep; + struct rmnet_endpoint *ep; + struct rmnet_port *port; + unsigned long bkt_ep; + u32 headroom; + + port = rmnet_get_port_rtnl(real_dev); + + headroom = sizeof(struct rmnet_map_header); + + if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) + headroom += sizeof(struct rmnet_map_dl_csum_trailer); + + hash_for_each_safe(port->muxed_ep, bkt_ep, tmp_ep, ep, hlnode) { + if (ep->egress_dev->mtu > (real_dev->mtu - headroom)) + return -1; + } + + return 0; +} + +int rmnet_vnd_update_dev_mtu(struct rmnet_port *port, + struct net_device *real_dev) +{ + struct hlist_node *tmp_ep; + struct rmnet_endpoint *ep; + unsigned long bkt_ep; + u32 headroom; + + headroom = sizeof(struct rmnet_map_header); + + if (port->data_format & RMNET_FLAGS_INGRESS_MAP_CKSUMV4) + headroom += sizeof(struct rmnet_map_dl_csum_trailer); + + hash_for_each_safe(port->muxed_ep, bkt_ep, tmp_ep, ep, hlnode) { + if (ep->egress_dev->mtu <= (real_dev->mtu - headroom)) + continue; + + if (rmnet_vnd_change_mtu(ep->egress_dev, + real_dev->mtu - headroom)) + return -1; + } + + return 0; +} \ No newline at end of file diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h index 4967f34..dc3a444 100644 --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.h @@ -18,4 +18,7 @@ int rmnet_vnd_dellink(u8 id, struct rmnet_port *port, void rmnet_vnd_rx_fixup(struct sk_buff *skb, struct net_device *dev); void rmnet_vnd_tx_fixup(struct sk_buff *skb, struct net_device *dev); void rmnet_vnd_setup(struct net_device *dev); +int rmnet_vnd_validate_real_dev_mtu(struct net_device *real_dev); +int rmnet_vnd_update_dev_mtu(struct rmnet_port *port, + struct net_device *real_dev); #endif /* _RMNET_VND_H_ */ -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Subash, On Tue, 8 Dec 2020 at 09:19, <subashab@codeaurora.org> wrote: > > >> What about just returning an error on NETDEV_PRECHANGEMTU notification > >> to prevent real device MTU change while virtual rmnet devices are > >> linked? Not sure there is a more proper and thread safe way to manager > >> that otherwise. > > > > Can't you copy what vlan devices do? That'd seem like a reasonable and > > well tested precedent, no? > > Could you try this patch. I've tried addressing most of the conditions > here. > I haven't seen any issues with updating the MTU when rmnet devices are > linked. Thanks, could you submit it as new regular patch, I'll give you my tested-by. Regards, Loic
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c index d58b51d..d1d7328 100644 --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c @@ -60,9 +60,14 @@ static netdev_tx_t rmnet_vnd_start_xmit(struct sk_buff *skb, static int rmnet_vnd_change_mtu(struct net_device *rmnet_dev, int new_mtu) { + struct rmnet_priv *priv = netdev_priv(rmnet_dev); + if (new_mtu < 0 || new_mtu > RMNET_MAX_PACKET_SIZE) return -EINVAL; + if (priv->real_dev && (new_mtu + RMNET_NEEDED_HEADROOM) > priv->real_dev->mtu) + return -EINVAL; + rmnet_dev->mtu = new_mtu; return 0; } @@ -242,6 +247,9 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev, priv->real_dev = real_dev; + /* Align default MTU with real_dev MTU */ + rmnet_vnd_change_mtu(rmnet_dev, real_dev->mtu - RMNET_NEEDED_HEADROOM); + rc = register_netdevice(rmnet_dev); if (!rc) { ep->egress_dev = rmnet_dev;
A submitted qmap/rmnet packet size can not be larger than the linked interface (real_dev) MTU. This patch ensures that the rmnet virtual iface MTU is configured according real device capability. Signed-off-by: Loic Poulain <loic.poulain@linaro.org> --- drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 8 ++++++++ 1 file changed, 8 insertions(+) -- 2.7.4