From: subashab@codeaurora.org
To: Jakub Kicinski <kuba@kernel.org>, Loic Poulain <loic.poulain@linaro.org>
Cc: stranche@codeaurora.org, Network Development <netdev@vger.kernel.org>
Subject: Re: [PATCH] net: rmnet: Adjust virtual device MTU on real device capability
Date: Tue, 08 Dec 2020 01:19:09 -0700 [thread overview]
Message-ID: <c7be03c227efc3405f4c9cd14e52d061@codeaurora.org> (raw)
In-Reply-To: <20201207121654.17fac0ef@kicinski-fedora-pc1c0hjn.DHCP.thefacebook.com>
>> 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
next prev parent reply other threads:[~2020-12-08 8:20 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2020-12-08 14:50 ` Loic Poulain
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c7be03c227efc3405f4c9cd14e52d061@codeaurora.org \
--to=subashab@codeaurora.org \
--cc=kuba@kernel.org \
--cc=loic.poulain@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=stranche@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.