From: subashab@codeaurora.org
To: "Bjørn Mork" <bjorn@mork.no>,
"Aleksander Morgado" <aleksander@aleksander.es>,
"Daniele Palmas" <dnlplm@gmail.com>
Cc: Network Development <netdev@vger.kernel.org>,
Sean Tranchetti <stranche@codeaurora.org>
Subject: Re: RMNET QMAP data aggregation with size greater than 16384
Date: Mon, 09 Aug 2021 15:40:41 -0600 [thread overview]
Message-ID: <394353d6f31303c64b0d26bc5268aca7@codeaurora.org> (raw)
In-Reply-To: <87sfzlplr2.fsf@miraculix.mork.no>
> No need for () around a constant, is there?
>
> Either I'm blind, or you don't actuelly change the rx_urb_size for the
> mux and pass through modes?
>
I seem to have missed this and the other stuff you have pointed out.
Can you please review this update-
>
> I'd also prefer this to reset back to syncing with hard_mtu if/when
> muxing or passthrough is disabled. Calling usbnet_change_mtu() won't
> do
> that. It doesn't touch rx_urb_size if it is different from hard_mtu.
>
> I also think that it might be useful to keep the mtu/hard_mtu control,
> wouldn't it?
>
>
> Something like
>
> old_rx_urb_size = dev->rx_urb_size;
> if (mux|passthrough)
> dev->rx_urb_size = MAP_DL_URB_SIZE;
> else
> dev->rx_urb_size = dev->hard_mtu;
> if (dev->rx_urb_size > old_rx_urb_size)
> unlink_urbs etc;
> return usbnet_change_mtu(net, new_mtu);
>
> should do that, I think. Completely untested....
>
> And add it to the (!qmimux_has_slaves(dev)) cas in del_mux_store() too.
>
Assuming this patch doesn't have too many other issues, can I request
Aleksander / Daniele to try this out.
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 6a2e4f8..4676544 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -75,6 +75,8 @@ struct qmimux_priv {
u8 mux_id;
};
+#define MAP_DL_URB_SIZE 32768
+
static int qmimux_open(struct net_device *dev)
{
struct qmimux_priv *priv = netdev_priv(dev);
@@ -303,6 +305,39 @@ static void qmimux_unregister_device(struct
net_device *dev,
dev_put(real_dev);
}
+static int qmi_wwan_change_mtu(struct net_device *net, int new_mtu)
+{
+ struct usbnet *dev = netdev_priv(net);
+ struct qmi_wwan_state *info = (void *)&dev->data;
+ int old_rx_urb_size = dev->rx_urb_size;
+
+ /* mux and pass through modes use a fixed rx_urb_size and the
value
+ * is independent of mtu
+ */
+ if (info->flags & (QMI_WWAN_FLAG_MUX |
QMI_WWAN_FLAG_PASS_THROUGH)) {
+ if (old_rx_urb_size == MAP_DL_URB_SIZE)
+ return 0;
+
+ if (old_rx_urb_size < MAP_DL_URB_SIZE) {
+ dev->rx_urb_size = MAP_DL_URB_SIZE;
+
+ usbnet_pause_rx(dev);
+ usbnet_unlink_rx_urbs(dev);
+ usbnet_resume_rx(dev);
+ usbnet_update_max_qlen(dev);
+ }
+
+ return 0;
+ }
+
+ /* rawip mode uses existing logic of setting rx_urb_size based
on mtu.
+ * rx_urb_size will be updated within usbnet_change_mtu only if
it is
+ * equal to existing hard_mtu
+ */
+ dev->rx_urb_size = dev->hard_mtu;
+ return usbnet_change_mtu(net, new_mtu);
+}
+
static void qmi_wwan_netdev_setup(struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
@@ -326,7 +361,7 @@ static void qmi_wwan_netdev_setup(struct net_device
*net)
}
/* recalculate buffers after changing hard_header_len */
- usbnet_change_mtu(net, net->mtu);
+ qmi_wwan_change_mtu(net, net->mtu);
}
static ssize_t raw_ip_show(struct device *d, struct device_attribute
*attr, char *buf)
@@ -433,6 +468,7 @@ static ssize_t add_mux_store(struct device *d,
struct device_attribute *attr, c
if (!ret) {
info->flags |= QMI_WWAN_FLAG_MUX;
ret = len;
+ qmi_wwan_change_mtu(dev->net, dev->net->mtu);
}
err:
rtnl_unlock();
@@ -466,8 +502,11 @@ static ssize_t del_mux_store(struct device *d,
struct device_attribute *attr, c
}
qmimux_unregister_device(del_dev, NULL);
- if (!qmimux_has_slaves(dev))
+ if (!qmimux_has_slaves(dev)) {
info->flags &= ~QMI_WWAN_FLAG_MUX;
+ qmi_wwan_change_mtu(dev->net, dev->net->mtu);
+ }
+
ret = len;
err:
rtnl_unlock();
@@ -514,6 +553,8 @@ static ssize_t pass_through_store(struct device *d,
else
info->flags &= ~QMI_WWAN_FLAG_PASS_THROUGH;
+ qmi_wwan_change_mtu(dev->net, dev->net->mtu);
+
return len;
}
@@ -643,7 +684,7 @@ static const struct net_device_ops
qmi_wwan_netdev_ops = {
.ndo_stop = usbnet_stop,
.ndo_start_xmit = usbnet_start_xmit,
.ndo_tx_timeout = usbnet_tx_timeout,
- .ndo_change_mtu = usbnet_change_mtu,
+ .ndo_change_mtu = qmi_wwan_change_mtu,
.ndo_get_stats64 = dev_get_tstats64,
.ndo_set_mac_address = qmi_wwan_mac_addr,
.ndo_validate_addr = eth_validate_addr,
next prev parent reply other threads:[~2021-08-09 21:41 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-31 22:45 RMNET QMAP data aggregation with size greater than 16384 Aleksander Morgado
2021-08-05 19:10 ` subashab
2021-08-05 20:32 ` Aleksander Morgado
[not found] ` <CAGRyCJHYkH4_FvTzk7BFwjMN=iOTN_Y2=4ueY=s3rJMQO9j7uw@mail.gmail.com>
2021-08-05 21:01 ` Aleksander Morgado
2021-08-05 21:12 ` Daniele Palmas
2021-08-05 22:57 ` subashab
2021-08-06 14:08 ` Aleksander Morgado
2021-08-06 18:42 ` subashab
2021-08-06 19:58 ` Bjørn Mork
2021-08-06 20:22 ` Aleksander Morgado
2021-08-06 22:30 ` subashab
2021-08-07 10:55 ` Bjørn Mork
2021-08-09 21:40 ` subashab [this message]
2021-08-12 12:02 ` Daniele Palmas
2021-08-13 6:21 ` subashab
2021-08-13 6:25 ` Bjørn Mork
2021-09-03 13:55 ` Daniele Palmas
2021-09-08 6:21 ` subashab
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=394353d6f31303c64b0d26bc5268aca7@codeaurora.org \
--to=subashab@codeaurora.org \
--cc=aleksander@aleksander.es \
--cc=bjorn@mork.no \
--cc=dnlplm@gmail.com \
--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.