* [PATCH] net: rmnet: Adjust virtual device MTU on real device capability
@ 2020-12-03 17:40 Loic Poulain
2020-12-04 21:56 ` subashab
0 siblings, 1 reply; 6+ messages in thread
From: Loic Poulain @ 2020-12-03 17:40 UTC (permalink / raw)
To: subashab, stranche; +Cc: netdev, kuba, Loic Poulain
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;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: rmnet: Adjust virtual device MTU on real device capability
2020-12-03 17:40 [PATCH] net: rmnet: Adjust virtual device MTU on real device capability Loic Poulain
@ 2020-12-04 21:56 ` subashab
2020-12-07 16:45 ` Loic Poulain
0 siblings, 1 reply; 6+ messages in thread
From: subashab @ 2020-12-04 21:56 UTC (permalink / raw)
To: Loic Poulain; +Cc: stranche, netdev, kuba
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: rmnet: Adjust virtual device MTU on real device capability
2020-12-04 21:56 ` subashab
@ 2020-12-07 16:45 ` Loic Poulain
2020-12-07 20:16 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Loic Poulain @ 2020-12-07 16:45 UTC (permalink / raw)
To: subashab; +Cc: stranche, Network Development, Jakub Kicinski
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: rmnet: Adjust virtual device MTU on real device capability
2020-12-07 16:45 ` Loic Poulain
@ 2020-12-07 20:16 ` Jakub Kicinski
2020-12-08 8:19 ` subashab
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2020-12-07 20:16 UTC (permalink / raw)
To: Loic Poulain; +Cc: subashab, stranche, Network Development
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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: rmnet: Adjust virtual device MTU on real device capability
2020-12-07 20:16 ` Jakub Kicinski
@ 2020-12-08 8:19 ` subashab
2020-12-08 14:50 ` Loic Poulain
0 siblings, 1 reply; 6+ messages in thread
From: subashab @ 2020-12-08 8:19 UTC (permalink / raw)
To: Jakub Kicinski, Loic Poulain; +Cc: stranche, Network Development
>> 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: rmnet: Adjust virtual device MTU on real device capability
2020-12-08 8:19 ` subashab
@ 2020-12-08 14:50 ` Loic Poulain
0 siblings, 0 replies; 6+ messages in thread
From: Loic Poulain @ 2020-12-08 14:50 UTC (permalink / raw)
To: subashab; +Cc: Jakub Kicinski, stranche, Network Development
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-08 14:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03 17:40 [PATCH] net: rmnet: Adjust virtual device MTU on real device capability Loic Poulain
2020-12-04 21:56 ` subashab
2020-12-07 16:45 ` Loic Poulain
2020-12-07 20:16 ` Jakub Kicinski
2020-12-08 8:19 ` subashab
2020-12-08 14:50 ` Loic Poulain
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.