netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: "Bjørn Mork" <bjorn@mork.no>
Cc: Daniele Palmas <dnlplm@gmail.com>,
	David Miller <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Eric Dumazet <edumazet@google.com>,
	linux-usb@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH net-next 0/2] net: usb: qmi_wwan implement tx packets aggregation
Date: Wed, 19 Oct 2022 17:48:35 +0200	[thread overview]
Message-ID: <Y1AcU0CH/j69uvwx@kroah.com> (raw)
In-Reply-To: <87lepbsvls.fsf@miraculix.mork.no>

On Wed, Oct 19, 2022 at 05:04:31PM +0200, Bjørn Mork wrote:
> Daniele Palmas <dnlplm@gmail.com> writes:
> > I'm aware that rmnet should be the preferred way for qmap, but I think there's
> > still value in adding this feature to qmi_wwan qmap implementation since there
> > are in the field many users of that.
> >
> > Moreover, having this in mainline could simplify backporting for those who are
> > using qmi_wwan qmap feature but are stuck with old kernel versions.
> >
> > I'm also aware of the fact that sysfs files for configuration are not the
> > preferred way, but it would feel odd changing the way for configuring the driver
> > just for this feature, having it different from the previous knobs.
> 
> It's not just that it's not the preferred way.. I believe I promised
> that we wouldn't add anything more to this interface.  And then I broke
> that promise, promising that it would never happen again.  So much for
> my integrity.
> 
> This all looks very nice to me, and the results are great, and it's just
> another knob...
> 

Please no more sysfs files for stuff like this.  This turns into
vendor-specific random files that no one knows how to change over time
with no way to know what userspace tools are accessing them, or if even
anyone is using them at all.

Shouldn't there be standard ethtool apis for this?

> But I don't think we can continue adding this stuff.  The QMAP handling
> should be done in the rmnet driver. Unless there is some reason it can't
> be there? Wouldn't the same code work there?

rmnet would be better, but again, no new sysfs files please,

thanks,

greg k-h

  reply	other threads:[~2022-10-19 15:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-19 13:25 [PATCH net-next 0/2] net: usb: qmi_wwan implement tx packets aggregation Daniele Palmas
2022-10-19 13:25 ` [PATCH net-next 1/2] net: usb: qmi_wwan: implement qmap uplink tx aggregation Daniele Palmas
2022-10-19 13:25 ` [PATCH net-next 2/2] Documentation: ABI: sysfs-class-net-qmi: document tx aggregation files Daniele Palmas
2022-10-19 15:04 ` [PATCH net-next 0/2] net: usb: qmi_wwan implement tx packets aggregation Bjørn Mork
2022-10-19 15:48   ` Greg KH [this message]
2022-10-20  0:55     ` Jakub Kicinski
2022-10-19 18:04   ` Daniele Palmas

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=Y1AcU0CH/j69uvwx@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=bjorn@mork.no \
    --cc=davem@davemloft.net \
    --cc=dnlplm@gmail.com \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).