Netdev List
 help / color / mirror / Atom feed
From: Jeremy Kerr <jk@codeconstruct.com.au>
To: Adam Young <admiyo@amperemail.onmicrosoft.com>,
	Adam Young	 <admiyo@os.amperecomputing.com>,
	Matt Johnston <matt@codeconstruct.com.au>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet	 <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni	 <pabeni@redhat.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Sudeep Holla <sudeep.holla@arm.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	 Huisong Li <lihuisong@huawei.com>
Subject: Re: [net-next v41] mctp pcc: Implement MCTP over PCC Transport
Date: Tue, 12 May 2026 12:07:55 +0800	[thread overview]
Message-ID: <71b7c8f40efc25fadb0aede6be888986364f2d53.camel@codeconstruct.com.au> (raw)
In-Reply-To: <9519a497-c10c-4abc-bcf9-414467f74ba3@amperemail.onmicrosoft.com>

Hi Adam,

> Sorry, I should have been more explicit here.  I am not certain what is 
> going to happen with fragmentation, so I want to be protected against
> future changes.

Nothing is going to happen with fragmentation. Your driver will only see
linear skbs in the ndo_start_xmit callback.

Adding the skb_linearize() there is unnecessary, and creates ambiguity
about the structure of the skbs that we're dealing with in that path.

> The check in validate_xmit_skb() is good, as it protects against the 
> current set up. So my option was to put a comment in here and hope both 
> changes happened together, or to just try and get this portion of the
> driver solid against the change.

I'm not clear on what changes you're referring to here?

> And I thought that was what you were suggesting in the comment. The 
> original comment sounded more like an "here is an optimization" instead 
> of "this is important enough to kick back"

It's less of an optimisation, and more removing something that is
unneeded, and potentially confusing ("why is the driver doing this?")?

> As for spacing, I get that there is a style, but it really should be 
> encoded in checkstyle.sh or something and automated.  My own tendency is 
> to put way too many spaces in to chunk things together, and I end up 
> going over-draconian on stripping them out to try and meet the expected 
> layout.

I'm not too fussed about the style at the level you have here, these are
suggestions to clean up if you're already re-rolling. My confusion is
that you had applied my feedback to one part of the code, but not to the
area I had commented on (which has the same style structure).

You also had some sashiko feedback on v40. I'm not sure whether all
items are relevant (I *think* you're OK for the first, for example), but
worth confirming:

https://sashiko.dev/#/patchset/20260508032953.337036-1-admiyo%40os.amperecomputing.com

Cheers,


Jeremy

  reply	other threads:[~2026-05-12  4:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-10 16:32 [net-next v41] mctp pcc: Implement MCTP over PCC Transport Adam Young
2026-05-11  2:19 ` Jeremy Kerr
2026-05-11 14:52   ` Adam Young
2026-05-12  4:07     ` Jeremy Kerr [this message]
2026-05-12 16:38       ` Adam Young

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=71b7c8f40efc25fadb0aede6be888986364f2d53.camel@codeconstruct.com.au \
    --to=jk@codeconstruct.com.au \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=admiyo@amperemail.onmicrosoft.com \
    --cc=admiyo@os.amperecomputing.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=lihuisong@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@codeconstruct.com.au \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sudeep.holla@arm.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