Message ID | 49E89E0D.5080000@gmail.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
Saikiran Madugula wrote: > 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. > > Signed-off-by: Saikiran Madugula <hummerbliss@gmail.com> > --- > net/bridge/br_netfilter.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c > index 3953ac4..941f702 100644 > --- a/net/bridge/br_netfilter.c > +++ b/net/bridge/br_netfilter.c > @@ -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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patrick McHardy wrote: > Saikiran Madugula wrote: >> 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. >> >> Signed-off-by: Saikiran Madugula <hummerbliss@gmail.com> >> --- >> net/bridge/br_netfilter.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c >> index 3953ac4..941f702 100644 >> --- a/net/bridge/br_netfilter.c >> +++ b/net/bridge/br_netfilter.c >> @@ -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. 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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Saikiran Madugula wrote: > Patrick McHardy wrote: >>> - 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. Good point. Yes, everything related to fragmenting is only needed with NF_CONNTRACK, so an additional ifdef makes sense. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c index 3953ac4..941f702 100644 --- a/net/bridge/br_netfilter.c +++ b/net/bridge/br_netfilter.c @@ -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);
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. Signed-off-by: Saikiran Madugula <hummerbliss@gmail.com> --- net/bridge/br_netfilter.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)