netdev - Re: [PATCH ethtool] ethtool: Add support for WAKE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8f93154f-ab52-713c-e306-5a79b1dbfe47@gmail.com>
Date:   Fri, 3 Aug 2018 12:58:12 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, linville@...driver.com, andrew@...n.ch,
        vivien.didelot@...oirfairelinux.com
Subject: Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER

On 08/03/2018 12:07 PM, David Miller wrote:
> From: Florian Fainelli <f.fainelli@...il.com>
> Date: Fri, 3 Aug 2018 10:57:13 -0700
> 
>> Does the current approach of specifying a bitmask of filters looks
>> reasonable to you though?
> 
> So, in order to answer that, I need some clarification.
> 
> The mask, as I see it, is a bit map of 48 possible positions
> (SOPASS_MAX * bits_per_byte).  How do these bits map to individual
> rxnfc entries?

Correct about the size, it is 48-bits, each bit indeed does map to a
filter location. So if you programmed a filter a location 1, you would
pass 0x2 as the wake-on filter bitmask, etc.

> 
> Are they locations?  If so, how are special locations handled?
> 
> What about "special" locations, where the driver and/or hardware
> are supposed to decide the location based upon the "special" type
> used?

I would not think they require special handling because the process is
kind of two step right now:

- first you program the desired filter (special location or not) and you
obtain an unique ID back
- second you program the desired filter mask with that ID as a bit
position that must be set

So the special location handling was kind of done by the kernel/driver
on the first filter insertion and you just pass that unique filter ID
around.

The reason why it was done as a two step process was largely because the
DSA switch driver, which is the one supporting the filter programming is
a discrete driver from the SYSTEM PORT driver which supports the
wake-on-filter thing. The two do communicate with one another through
the means of the DSA layer though.

Now that I think about it some more, see below, I prefer you approach
since it eliminates the "passing that ID around" step.

> 
> If you considered the following, and you explained why it won't
> work, I apologize.  But I'm wondering why you just don't find
> some way to specify this as a boolean of the flow spec in the
> rxnfc request or similar?
> 
> That, at least semantically, seems to avoids several issues.  And it
> is unambiguous what flow rule the wake filter boolean applies to.
> 
> Right?

Yes, it would actually remove the need for having to specify a storage
location between user-space and kernel space and we would also be able
to valid ahead of time that we are not overflowing the wake-on-LAN
filter capacity. For instance, in the current HW, you can program 128
filters through the switch, but only 8 of those could be wake-up capable
at the CPU/management (SYSTEM PORT) level.

Let me cook something that does just that and re-post.

Thanks for your feedback!
-- 
Florian