On 08/03/2018 12:07 PM, David Miller wrote: > From: Florian Fainelli <f.faine...@gmail.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