Adam Osuchowski wrote: > Stephen Hemminger wrote: > >>It would be better to account for the tag in the length check. >>Something like >> if (skb->protocol == htons(ETH_P_IP) && >> skb->len > skb->dev->mtu - (IS_VLAN_IP(skb) ? VLAN_HLEN : 0) && >> !skb_is_gso(skb)) >> return ip_fragment ... > > > It isn't good solution because one of IS_VLAN_IP() necessary condition is > > skb->protocol == htons(ETH_P_8021Q) > > which is, of course, mutually exclusive with > > skb->protocol == htons(ETH_P_IP) > > from br_nf_dev_queue_xmit(). IMHO, one should check length of ETH_P_IP > and ETH_P_8021Q frames separately: > > if (((skb->protocol == htons(ETH_P_IP) && skb->len > skb->dev->mtu) || > (IS_VLAN_IP(skb) && skb->len > skb->dev->mtu - VLAN_HLEN)) && > !skb_is_gso(skb)) > return ip_fragment ...
net/8021q ignores the VLAN header overhead, so we should probably do the same here for consistency. Using IS_VLAN_IP (and IS_PPPOE_IP for current -rc) looks fine, additionally we should probably also check for skb->nfct != NULL to make sure that at least without connection tracking the bridge doesn't perform fragmentation. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
