Allow fragmentation of VLAN packets traversing a bridge.
Subsystems (4)
PATCH 1/1 Allow fragmentation of VLAN packets traversing a bridge.
br_nf_dev_queue_xmit only checks for ETH_P_IP packets for fragmenting but not VLAN packets. This results in dropping of large VLAN packets. This can be observed when connection tracking is enabled. Connection tracking re-assembles fragmented packets, and these have to be re-fragmented when transmitting out.
--- net/bridge/br_netfilter.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
net/bridge/br_netfilter.c +1 -1
@@ -790,7 +790,7 @@ static unsigned int br_nf_local_out(unsigned int hook, struct sk_buff *skb,static int br_nf_dev_queue_xmit (struct sk_buff *skb) { -if (skb->protocol ==htons (ETH_P_IP ) && +if ((skb->protocol ==htons (ETH_P_IP ) ||IS_VLAN_IP (skb)) && skb->len > skb->dev->mtu && !skb_is_gso (skb))return ip_fragment (skb, br_dev_queue_push_xmit);
Signature · 1 line
1.5.6.3
Quoted 15 lines · Saikiran Madugula
br_nf_dev_queue_xmitonly checks forETH_P_IPpackets for fragmenting but not VLAN packets. This results in dropping of large VLAN packets. This can be observed when connection tracking is enabled. Connection tracking re-assembles fragmented packets, and these have to be re-fragmented when transmitting out.Signed-off-by Saikiran Madugula <[email protected]>--- net/bridge/br_netfilter.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)net/bridge/br_netfilter.c +0
-0
net/bridge/br_netfilter.c static unsigned int br_nf_local_out(unsigned int hook, struct sk_buff *skb, · line 796
Quoted 8 lines
net/bridge/br_netfilter.c +1
-1@@ -790,7 +790,7 @@ static unsigned int br_nf_local_out(unsigned int hook, struct sk_buff *skb,static int br_nf_dev_queue_xmit (struct sk_buff *skb) { -if (skb->protocol ==htons (ETH_P_IP ) && +if ((skb->protocol ==htons (ETH_P_IP ) ||IS_VLAN_IP (skb)) && skb->len > skb->dev->mtu && !skb_is_gso (skb))return ip_fragment (skb, br_dev_queue_push_xmit);
Please add an additional check for skb->nfct != NULL to make sure that this only refragments packets defragmented by conntrack.
Quoted 21 lines · Patrick McHardy
Quoted 20 lines · Saikiran Madugula
br_nf_dev_queue_xmitonly checks forETH_P_IPpackets for fragmenting but not
VLAN packets. This results in dropping of large VLAN packets. This can be observed when connection tracking is enabled. Connection tracking re-assembles
fragmented packets, and these have to be re-fragmented when transmitting out.Signed-off-by Saikiran Madugula <[email protected]>--- net/bridge/br_netfilter.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)net/bridge/br_netfilter.c +0
-0@@ -790,7 +790,7 @@ static unsigned int br_nf_local_out(unsigned int hook, struct sk_buff *skb,
Quoted 10 lines
Quoted 7 lines
net/bridge/br_netfilter.c +1
-1static int br_nf_dev_queue_xmit (struct sk_buff *skb) { -if (skb->protocol ==htons (ETH_P_IP ) && +if ((skb->protocol ==htons (ETH_P_IP ) ||IS_VLAN_IP (skb)) && skb->len > skb->dev->mtu && !skb_is_gso (skb))return ip_fragment(skb, br_dev_queue_push_xmit);Please add an additional check for
skb->nfct!= NULL to make sure that this only refragments packets defragmented by conntrack.
Thanks for the feedback, skb->nfct is present only if CONFIG_NF_CONNTRACK or CONFIG_NF_CONNTRACK_MODULE is defined. Was wondering if the entire check before ip_fragment is necessary if NF_CONNTRACK is not defined. If not, I will post updated patch as per your suggestion.
Quoted 6 lines · Saikiran Madugula
Quoted 5 lines · Patrick McHardy
Quoted 5 lines
net/bridge/br_netfilter.c +1
-1-if (skb->protocol ==htons (ETH_P_IP ) && +if ((skb->protocol ==htons (ETH_P_IP ) ||IS_VLAN_IP (skb)) && skb->len > skb->dev->mtu && !skb_is_gso (skb))return ip_fragment (skb, br_dev_queue_push_xmit);
Quoted 7 lines
Quoted 2 lines
Please add an additional check for
skb->nfct!= NULL to make sure that this only refragments packets defragmented by conntrack.Thanks for the feedback,
skb->nfctis present only ifCONFIG_NF_CONNTRACKorCONFIG_NF_CONNTRACK_MODULEis defined. Was wondering if the entire check beforeip_fragmentis necessary ifNF_CONNTRACKis not defined. If not, I will post updated patch as per your suggestion.
Good point. Yes, everything related to fragmenting is only needed with NF_CONNTRACK, so an additional ifdef makes sense.
PATCH 1/1 Allow fragmentation of VLAN packets traversing a bridge.
br_nf_dev_queue_xmit only checks for ETH_P_IP packets for fragmenting but not VLAN packets. This results in dropping of large VLAN packets. This can be observed when connection tracking is enabled. Connection tracking re-assembles fragmented packets, and these have to re-fragmented when transmitting out. Also, make sure only refragmented packets are defragmented as per suggestion from Patrick McHardy.
--- net/bridge/br_netfilter.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)
net/bridge/br_netfilter.c +9 -1
@@ -788,15 +788,23 @@ static unsigned int br_nf_local_out(unsigned int hook, struct sk_buff *skb,return NF_STOLEN ; } +#if defined(CONFIG_NF_CONNTRACK_IPV4 ) || defined(CONFIG_NF_CONNTRACK_IPV4_MODULE )static int br_nf_dev_queue_xmit (struct sk_buff *skb) { -if (skb->protocol ==htons (ETH_P_IP ) && +if (skb->nfct !=NULL && + (skb->protocol ==htons (ETH_P_IP ) ||IS_VLAN_IP (skb)) && skb->len > skb->dev->mtu && !skb_is_gso (skb))return ip_fragment (skb, br_dev_queue_push_xmit);else return br_dev_queue_push_xmit (skb); } +#else +static int br_nf_dev_queue_xmit (struct sk_buff *skb) +{ +return br_dev_queue_push_xmit (skb); +} +#endif /* PF_BRIDGE/POST_ROUTING ********************************************/ static unsigned int br_nf_post_routing(unsigned int hook, struct sk_buff *skb,
Signature · 1 line
1.5.6.3
Patrick McHardy
AppliedQuoted 6 lines · Saikiran Madugula
br_nf_dev_queue_xmitonly checks forETH_P_IPpackets for fragmenting but not VLAN packets. This results in dropping of large VLAN packets. This can be observed when connection tracking is enabled. Connection tracking re-assembles fragmented packets, and these have to re-fragmented when transmitting out. Also, make sure only refragmented packets are defragmented as per suggestion from Patrick McHardy.
Applied, thanks Saikiran.