Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <b3cd61506080143f571d6286223ae33c8bd02c3a.camel@sipsolutions.net>
Subject: Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Jiri Pirko <jiri@resnulli.us>, Michal Kubecek <mkubecek@suse.cz>
Cc:     David Miller <davem@davemloft.net>, netdev@vger.kernel.org,
        Jakub Kicinski <jakub.kicinski@netronome.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        John Linville <linville@tuxdriver.com>,
        Stephen Hemminger <stephen@networkplumber.org>,
        linux-kernel@vger.kernel.org
Date:   Wed, 03 Jul 2019 15:44:57 +0200
In-Reply-To: <20190703114933.GW2250@nanopsycho> (sfid-20190703_134935_025840_EF167268)
References: <cover.1562067622.git.mkubecek@suse.cz>
         <cb614bebee1686293127194e8f7ced72955c7c7f.1562067622.git.mkubecek@suse.cz>
         <20190703114933.GW2250@nanopsycho> (sfid-20190703_134935_025840_EF167268)
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 2019-07-03 at 13:49 +0200, Jiri Pirko wrote:
> 
> > +Value and mask must have length at least ETHTOOL_A_BITSET_SIZE bits rounded up
> > +to a multiple of 32 bits. They consist of 32-bit words in host byte order,
> 
> Looks like the blocks are similar to NLA_BITFIELD32. Why don't you user
> nested array of NLA_BITFIELD32 instead?

That would seem kind of awkward to use, IMHO.

Perhaps better to make some kind of generic "arbitrary size bitfield"
attribute type?

Not really sure we want the complexity with _LIST and _SIZE, since you
should always be able to express it as _VALUE and _MASK, right?

Trying to think how we should express this best - bitfield32 is just a
mask/value struct, for arbitrary size I guess we *could* just make it
kind of a binary with arbitrary length that must be a multiple of 2
bytes (or 2 u32-bit-words?) and then the first half is the value and the
second half is the mask? Some more validation would be nicer, but having
a generic attribute that actually is nested is awkward too.

johannes