Avinash Duduskar <[email protected]> writes: >> > + if (flags & BPF_FIB_LOOKUP_VLAN) >> > + return -EINVAL; >> > + >> >> This is fine, but we should probably reject the input flag as well in >> the next patch (for symmetry). > > I dug into this and I don't think the two are symmetric. The egress > reject is right for exactly the reason you gave: in tc you can redirect > to the VLAN device directly, so reducing the egress to the physical > parent is only needed for XDP. But that is a transmit argument, and > VLAN_INPUT never touches the egress or redirect side. It only sets the > lookup's ingress (flowi_iif), which picks the iif policy rule and the VRF > table, and there is no XDP-only constraint there for the symmetry to > mirror. > > tc also has a real user for it. In __netif_receive_skb_core() the tcx > ingress hook runs before vlan_do_receive() demuxes the frame, so a clsact > program on the physical port sees a tagged frame with skb->dev still the > physical device and the tag in skb->vlan_tci. That is exactly the > physical-ifindex-plus-tag input VLAN_INPUT takes, and it wants the > subinterface-scoped answer. The 2/3 selftest already runs the VLAN_INPUT > cases on the tc path, including the VRF-table-selection ones, and they > pass, so this isn't a theoretical tc path. > > So I would keep VLAN_INPUT allowed on both. If you would rather hold a > uniform "both VLAN flags are XDP-only" line under the same > restrict-now-relax-later rule as the TBID and OUTPUT combos, I will add > the reject, but unlike the egress case it removes a working tc path > rather than one tc cannot use. Happy either way, just let me know.
Hmm, OK, I'm fine with keeping it available for TC then. -Toke
