All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: Vladimir Oltean <olteanv@gmail.com>,
	"Kitszel, Przemyslaw" <przemyslaw.kitszel@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: [PATCH net-next 3/8] lib: packing: add pack_fields() and unpack_fields()
Date: Thu, 24 Oct 2024 09:38:48 -0700	[thread overview]
Message-ID: <38f27382-3c6d-4677-9d59-4d08104f1131@intel.com> (raw)
In-Reply-To: <20241024134902.xe7kd4t7yoy2i4xj@skbuf>



On 10/24/2024 6:49 AM, Vladimir Oltean wrote:
> I just want to say that I don't have any alternative proposals, nor will I
> explore your sparse suggestion. I don't know enough about sparse to judge
> whether something as 'custom' as the packing API is in scope for its
> check_call_instruction() infrastructure, how well will that solution
> deal with internal kernel API changes down the line, and I don't have
> the time to learn enough to prototype something to find the maintainers'
> answer to these questions, either. I strongly prefer to have the static
> checks inside the kernel, together with the packing() API itself, so it
> can be more easily altered.
> 
> Obviously you're still free to wait for more opinions and suggestions,
> or to experiment with the sparse idea yourself.
> 

I also have some thought about trying to catch this in a coccinelle
script. That has the trade-off that its only caught by running the
spatch/coccinelle scripts, but it would completely eliminate the need to
modify Kbuild at all.

I'm going to try and experiment with that direction and see if its feasible.

> Honestly, my opinion is that if we can avoid messing too much with the
> top-level Kbuild file, this pretty much enters "no one really cares"
> territory, as long as the code is generated only for the pack_fields()
> users. This is, in fact, one of the reasons why the patch I attached
> earlier compiles and runs the code-gen only when PACKING_CHECK_FIELDS
> is defined.

If I can't make that work today, I'll send a v2 with the
PACKING_CHECK_FIELDS and the other cleanups applied.

  reply	other threads:[~2024-10-24 16:38 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-11 18:48 [PATCH net-next 0/8] lib: packing: introduce and use (un)pack_fields Jacob Keller
2024-10-11 18:48 ` [PATCH net-next 1/8] lib: packing: create __pack() and __unpack() variants without error checking Jacob Keller
2024-10-14 14:27   ` Vladimir Oltean
2024-10-14 18:52     ` Jacob Keller
2024-10-19 12:45   ` Vladimir Oltean
2024-10-22 19:12     ` Jacob Keller
2024-10-11 18:48 ` [PATCH net-next 2/8] lib: packing: demote truncation error in pack() to a warning in __pack() Jacob Keller
2024-10-11 18:48 ` [PATCH net-next 3/8] lib: packing: add pack_fields() and unpack_fields() Jacob Keller
2024-10-15 19:19   ` Jacob Keller
2024-10-16 13:02   ` Przemek Kitszel
2024-10-16 13:40     ` Vladimir Oltean
2024-10-16 22:31       ` Keller, Jacob E
2024-10-18 21:50         ` Jacob Keller
2024-10-19 12:20           ` Vladimir Oltean
2024-10-22 19:11             ` Jacob Keller
2024-10-24 13:49               ` Vladimir Oltean
2024-10-24 16:38                 ` Jacob Keller [this message]
2024-10-24 20:14                   ` Jacob Keller
2024-10-24 20:30                     ` Vladimir Oltean
2024-10-11 18:48 ` [PATCH net-next 4/8] ice: remove int_q_state from ice_tlan_ctx Jacob Keller
2024-10-11 18:48 ` [PATCH net-next 5/8] ice: use <linux/packing.h> for Tx and Rx queue context data Jacob Keller
2024-10-19 13:39   ` Vladimir Oltean
2024-10-22 19:14     ` Jacob Keller
2024-10-11 18:48 ` [PATCH net-next 6/8] ice: reduce size of queue context fields Jacob Keller
2024-10-11 18:48 ` [PATCH net-next 7/8] ice: move prefetch enable to ice_setup_rx_ctx Jacob Keller
2024-10-11 18:48 ` [PATCH net-next 8/8] ice: cleanup Rx queue context programming functions Jacob Keller

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=38f27382-3c6d-4677-9d59-4d08104f1131@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=anthony.l.nguyen@intel.com \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=vladimir.oltean@nxp.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 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.