| Message ID | 1560159846-29933-1-git-send-email-wenxu@ucloud.cn |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] bridge: Set the pvid for untaged packet before prerouting | expand |
On 10/06/2019 12:44, wenxu@ucloud.cn wrote: > From: wenxu <wenxu@ucloud.cn> > > bridge vlan add dev veth1 vid 200 pvid untagged > bridge vlan add dev veth2 vid 200 pvid untagged > > nft add table bridge firewall > nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; } > nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 } > > As above set the bridge port with pvid, the received packet don't contain > the vlan tag which means the packet should belong to vlan 200 through pvid. > User can do conntrack base base on vlan id and map the vlan id to zone id > in the prerouting hook. > > Signed-off-by: wenxu <wenxu@ucloud.cn> > --- > net/bridge/br_input.c | 7 +++++++ > 1 file changed, 7 insertions(+) > Nacked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Hi, I don't think this is a good idea for a few reasons: - duplicating code (pvid insertion by __allowed_ingress) - adding 2 new tests in the fast path (even 3 in the vlan filtering case) - issue can be solved with current state by using different config Why do you need the vid to be set when you can assume that all the traffic from that port belongs to the pvid vlan ? In this case you can match the port ifindex for example and associate the zones based on that. Another approach - you can insert the vlan by tc's vlan action on ingress, you'll get the same effect. Overall this looks like an issue solvable by different config instead of adding new tests in the fast path and increasing the complexity of the bridge input code for little value. Cheers, Nik
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index 21b74e7..31b44bc 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -341,6 +341,13 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb) } forward: + if (br_opt_get(p->br, BROPT_VLAN_ENABLED) && !skb_vlan_tag_present(skb)) { + u16 pvid = br_get_pvid(nbp_vlan_group_rcu(p)); + + if (pvid) + __vlan_hwaccel_put_tag(skb, p->br->vlan_proto, pvid); + } + switch (p->state) { case BR_STATE_FORWARDING: case BR_STATE_LEARNING: