| Message ID | 1653548252-2602-1-git-send-email-wenxu@chinatelecom.cn |
|---|---|
| State | Deferred |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [nf-next,v2,1/3] nf_flow_table_offload: offload the vlan encap in the flowtable | expand |
On Thu, May 26, 2022 at 02:57:30AM -0400, wenxu@chinatelecom.cn wrote: [...] > diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c > index b350fe9..5da651d 100644 > --- a/net/netfilter/nf_flow_table_ip.c > +++ b/net/netfilter/nf_flow_table_ip.c > @@ -291,6 +291,23 @@ static bool nf_flow_skb_encap_protocol(const struct sk_buff *skb, __be16 proto, > return false; > } > > +static void nf_flow_encap_push(struct sk_buff *skb, > + struct flow_offload_tuple_rhash *tuplehash) > +{ > + int i; > + > + for (i = 0; i < tuplehash->tuple.encap_num; i++) { > + switch (tuplehash->tuple.encap[i].proto) { > + case htons(ETH_P_8021Q): > + case htons(ETH_P_8021AD): > + skb_vlan_push(skb, Nit: skb_vlan_push() might fail. > + tuplehash->tuple.encap[i].proto, > + tuplehash->tuple.encap[i].id); > + break; > + } > + } > +} If I understand correctly, the goal of this patchset is to move the existing vlan and ppp support to use the XMIT_DIRECT path? So this already works but you would prefer to not use XMIT_NEIGH? The scenarios you describe already work fine with the existing codebase? I am assuming 'eth' provides Internet access? You refer to this in the patch description: br0.100-->br0(vlan filter enable)-->eth br0(vlan filter enable)-->eth br0(vlan filter disable)-->eth.100-->eth
>On Thu, May 26, 2022 at 02:57:30AM -0400, wenxu@chinatelecom.cn wrote: >[...] >> diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c >> index b350fe9..5da651d 100644 >> --- a/net/netfilter/nf_flow_table_ip.c >> +++ b/net/netfilter/nf_flow_table_ip.c >> @@ -291,6 +291,23 @@ static bool nf_flow_skb_encap_protocol(const struct sk_buff *skb, __be16 proto, >> return false; >> } >> >> +static void nf_flow_encap_push(struct sk_buff *skb, >> + struct flow_offload_tuple_rhash *tuplehash) >> +{ >> + int i; >> + >> + for (i = 0; i < tuplehash->tuple.encap_num; i++) { >> + switch (tuplehash->tuple.encap[i].proto) { >> + case htons(ETH_P_8021Q): >> + case htons(ETH_P_8021AD): >> + skb_vlan_push(skb, > >Nit: skb_vlan_push() might fail. > >> + tuplehash->tuple.encap[i].proto, >> + tuplehash->tuple.encap[i].id); >> + break; >> + } >> + } >> +} > >If I understand correctly, the goal of this patchset is to move the >existing vlan and ppp support to use the XMIT_DIRECT path? > >So this already works but you would prefer to not use XMIT_NEIGH? > >The scenarios you describe already work fine with the existing >codebase? I am assuming 'eth' provides Internet access? You refer to >this in the patch description: The eth is the lower device of the bridge. router |------------| eth0-->br0 eth-internet Without this patch the packet come from eth-internet will always send through the router interface br0 with XMIT_NEIGH. With this patch the packet come from eth-internet will send through eth0 directly with XMIT_DIRECT(with vlan tag if need). So it can totally bypass the bridge process for ingress packet. > > br0.100-->br0(vlan filter enable)-->eth > br0(vlan filter enable)-->eth > br0(vlan filter disable)-->eth.100-->eth >
>> +static void nf_flow_encap_push(struct sk_buff *skb, >> + struct flow_offload_tuple_rhash *tuplehash) >> +{ >> + int i; >> + >> + for (i = 0; i < tuplehash->tuple.encap_num; i++) { >> + switch (tuplehash->tuple.encap[i].proto) { >> + case htons(ETH_P_8021Q): >> + case htons(ETH_P_8021AD): >> + skb_vlan_push(skb, > >Nit: skb_vlan_push() might fail. > The packet maybe modified. So maybe only drop this packet if skb_vlan_push failed?
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c index b350fe9..5da651d 100644 --- a/net/netfilter/nf_flow_table_ip.c +++ b/net/netfilter/nf_flow_table_ip.c @@ -291,6 +291,23 @@ static bool nf_flow_skb_encap_protocol(const struct sk_buff *skb, __be16 proto, return false; } +static void nf_flow_encap_push(struct sk_buff *skb, + struct flow_offload_tuple_rhash *tuplehash) +{ + int i; + + for (i = 0; i < tuplehash->tuple.encap_num; i++) { + switch (tuplehash->tuple.encap[i].proto) { + case htons(ETH_P_8021Q): + case htons(ETH_P_8021AD): + skb_vlan_push(skb, + tuplehash->tuple.encap[i].proto, + tuplehash->tuple.encap[i].id); + break; + } + } +} + static void nf_flow_encap_pop(struct sk_buff *skb, struct flow_offload_tuple_rhash *tuplehash) { @@ -417,6 +434,7 @@ static unsigned int nf_flow_queue_xmit(struct net *net, struct sk_buff *skb, ret = NF_STOLEN; break; case FLOW_OFFLOAD_XMIT_DIRECT: + nf_flow_encap_push(skb, &flow->tuplehash[!dir]); ret = nf_flow_queue_xmit(state->net, skb, tuplehash, ETH_P_IP); if (ret == NF_DROP) flow_offload_teardown(flow); @@ -678,6 +696,7 @@ static int nf_flow_tuple_ipv6(struct sk_buff *skb, const struct net_device *dev, ret = NF_STOLEN; break; case FLOW_OFFLOAD_XMIT_DIRECT: + nf_flow_encap_push(skb, &flow->tuplehash[!dir]); ret = nf_flow_queue_xmit(state->net, skb, tuplehash, ETH_P_IPV6); if (ret == NF_DROP) flow_offload_teardown(flow); diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c index a25c88b..bfe7a3a 100644 --- a/net/netfilter/nft_flow_offload.c +++ b/net/netfilter/nft_flow_offload.c @@ -123,13 +123,16 @@ static void nft_dev_path_info(const struct net_device_path_stack *stack, info->indev = NULL; break; } - if (!info->outdev) - info->outdev = path->dev; info->encap[info->num_encaps].id = path->encap.id; info->encap[info->num_encaps].proto = path->encap.proto; info->num_encaps++; - if (path->type == DEV_PATH_PPPOE) + if (path->type == DEV_PATH_PPPOE) { + if (!info->outdev) + info->outdev = path->dev; memcpy(info->h_dest, path->encap.h_dest, ETH_ALEN); + } + if (path->type == DEV_PATH_VLAN) + info->xmit_type = FLOW_OFFLOAD_XMIT_DIRECT; break; case DEV_PATH_BRIDGE: if (is_zero_ether_addr(info->h_source))