On Mon, Mar 01, 2021 at 05:08:52PM +0200, Vladimir Oltean wrote: > On Mon, Mar 01, 2021 at 03:36:15PM +0100, Michael Walle wrote: > > Ok, I see, so your proposed behavior is backed by the standards. But > > OTOH there was a summary by Markus of the behavior of other drivers: > > https://lore.kernel.org/netdev/20201119153751.ix73o5h4n6dgv...@ipetronik.com/ > > And a conclusion by Jakub: > > https://lore.kernel.org/netdev/20201112164457.6af0f...@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/#t > > And a propsed core change to disable vlan filtering with promisc mode. > > Do I understand you correctly, that this shouldn't be done either? > > > > Don't get me wrong, I don't vote against or in favor of this patch. > > I just want to understand the behavior. > > So you can involuntarily ignore a standard, or you can ignore it > deliberately. I can't force anyone to not ignore it in the latter case, > but indeed, now that I tried to look it up, I personally don't think > that promiscuity should disable VLAN filtering unless somebody comes up > with a good reason for which Linux should basically disregard IEEE 802.3. > In particular, Jakub seems to have been convinced in that thread by no > other argument except that other drivers ignore the standards too, which > I'm not sure is a convincing enough argument.
Admittedly, I am still not entirely convinced myself. I don't know why the other drivers do what they do, why they do it and whether that's correct. That is one of the reasons (next to quite a few issues I had with patching net/core) why I ungracefully abandoned the mentioned thread for now ( sorry and shame on me :-/ ). The main problem here could also just be that almost everybody _thinks_ that promiscuity means receiving all frames and no one is aware of the standards definition. In fact, I can't blame them, as the standard is hard to come by and not enjoyable to read, imho. And all secondary documentation I could find on the internet explain promiscuous mode as a "mode of operation" in which "the card accepts every Ethernet packet sent on the network" or similar. Even libpcap, which I consider the reference on network sniffing, thinks that "Promiscuous mode [...] sniffs all traffic on the wire." Thus I still think that this issue is also fixable by proper documentation of promiscuity. At least the meaning and guarantees of IFF_PROMISC in this kernel should be clearly defined - in one way or the other - such that users with different expectations can be directed there and drivers with different behavior can be fixed with that definition as justification. > > In my opinion, the fact that some drivers disable VLAN filtering should > be treated like a marginal condition, similar to how, when you set the > MTU on an interface to N octets, it might happen that it accepts packets > larger than N octets, but it isn't guaranteed. > > > I haven't had time to actually test this, but what if you do: > > - don't load the 8021q module (or don't enable kernel support) > > - enable promisc > > (1) > > - load 8021q module > > (2) > > - add a vlan interface > > (3) > > - add another vlan interface > > (4) > > > > What frames would you actually receive on the base interface > > in (1), (2), (3), (4) and what is the user expectation? > > I'd say its the same every time. (IIRC there is already some > > discrepancy due to the VLAN filter hardware offloading) > > The default value is: > ethtool -k eno0 | grep rx-vlan-filter > rx-vlan-filter: off > > so we receive all VLAN-tagged packets by default in enetc, unless VLAN > filtering is turned on. > > > > I chose option 2 because it was way simpler and was just as correct. > > > > Fair, but it will also put additional burden to the user to also > > disable the vlan filtering, right?. Otherwise it would just work. And > > it will waste CPU cycles for unwanted frames. > > Although your new patch version contains a new "(yet)" ;) > > True, nobody said it's optimal, but you can't make progress if you > always try to do things optimally the first time (but at least you > should do something that's not wrong). > Adding the dev_uc_add, dev_mc_add and vlan_vid_add calls to > net/sched/cls_flower.c doesn't seem an impossible task (especially since > all of them are refcounted, it should be pretty simple to avoid strange > interactions with other layers such as 8021q), but nonetheless, it just > wasn't (and still isn't) high enough on my list of priorities.
