From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9485CC4338F for ; Sat, 7 Aug 2021 10:55:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6792E60E09 for ; Sat, 7 Aug 2021 10:55:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231972AbhHGK4L (ORCPT ); Sat, 7 Aug 2021 06:56:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46538 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231687AbhHGK4K (ORCPT ); Sat, 7 Aug 2021 06:56:10 -0400 Received: from canardo.mork.no (canardo.mork.no [IPv6:2001:4641::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 55B2CC0613CF for ; Sat, 7 Aug 2021 03:55:53 -0700 (PDT) Received: from miraculix.mork.no ([IPv6:2a01:799:95f:ef0a:7f0c:624e:2eac:9b4]) (authenticated bits=0) by canardo.mork.no (8.15.2/8.15.2) with ESMTPSA id 177AtZWN005803 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Sat, 7 Aug 2021 12:55:36 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mork.no; s=b; t=1628333738; bh=UFmQOjxLHCFlD0dXZrH+NHtuOzsmGXbkLjV2rqqsSj0=; h=From:To:Cc:Subject:References:Date:Message-ID:From; b=V65hbHXMlzi78rsWU1hjG/cRfmAnN9TjTW5MvGFUDrcMsf/xVgnUbTDBnXYGYq6fn AgtLrvTJX1LSMS1TDbXwD6aDaL2neKrodh29XkWGbbHkBogsdgND+SB1UXKKLimGe0 VsKcVHMKxv8oBoJ7iSgS/xt3rN2qBmN5kKirsGlA= Received: from bjorn by miraculix.mork.no with local (Exim 4.94.2) (envelope-from ) id 1mCJzF-000Emt-Di; Sat, 07 Aug 2021 12:55:29 +0200 From: =?utf-8?Q?Bj=C3=B8rn_Mork?= To: subashab@codeaurora.org Cc: Aleksander Morgado , Daniele Palmas , Network Development , Sean Tranchetti Subject: Re: RMNET QMAP data aggregation with size greater than 16384 Organization: m References: <77b850933d9af8ddbc21f5908ca0764d@codeaurora.org> <13972ac97ffe7a10fd85fe03dc84dc02@codeaurora.org> <87bl6aqrat.fsf@miraculix.mork.no> <2c2d1204842f457bb0d0b2c4cd58847d@codeaurora.org> Date: Sat, 07 Aug 2021 12:55:29 +0200 In-Reply-To: <2c2d1204842f457bb0d0b2c4cd58847d@codeaurora.org> (subashab@codeaurora.org's message of "Fri, 06 Aug 2021 16:30:42 -0600") Message-ID: <87sfzlplr2.fsf@miraculix.mork.no> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Virus-Scanned: clamav-milter 0.103.2 at canardo X-Virus-Status: Clean Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org subashab@codeaurora.org writes: > Would something like this work- Looking pretty good to me, but I believe it needs some polishing. > diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c > index 6a2e4f8..c49827e 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) No need for () around a constant, is there? > static int qmimux_open(struct net_device *dev) > { > struct qmimux_priv *priv =3D netdev_priv(dev); > @@ -303,6 +305,33 @@ 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 =3D netdev_priv(net); > + struct qmi_wwan_state *info =3D (void *)&dev->data; > + int old_rx_urb_size =3D 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 =3D=3D MAP_DL_URB_SIZE) > + return 0; > + > + if (old_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 */ > + return usbnet_change_mtu(net, new_mtu); > +} Either I'm blind, or you don't actuelly change the rx_urb_size for the mux and pass through modes? 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 =3D dev->rx_urb_size; if (mux|passthrough) dev->rx_urb_size =3D MAP_DL_URB_SIZE; else dev->rx_urb_size =3D 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.... > + > static void qmi_wwan_netdev_setup(struct net_device *net) > { > struct usbnet *dev =3D netdev_priv(net); > @@ -326,7 +355,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 +462,7 @@ static ssize_t add_mux_store(struct device *d, > struct device_attribute *attr, c > if (!ret) { > info->flags |=3D QMI_WWAN_FLAG_MUX; > ret =3D len; > + qmi_wwan_change_mtu(dev->net, dev->net->mtu); > } > err: > rtnl_unlock(); > @@ -514,6 +544,8 @@ static ssize_t pass_through_store(struct device *d, > else > info->flags &=3D ~QMI_WWAN_FLAG_PASS_THROUGH; > > + qmi_wwan_change_mtu(dev->net, dev->net->mtu); > + > return len; > } And add it to the (!qmimux_has_slaves(dev)) cas in del_mux_store() too. Bj=C3=B8rn