[Bridge] [BUG] Dropping fragmented IP packets within VLAN frames on bridge

[Bridge] [BUG] Dropping fragmented IP packets within VLAN frames on bridge

Post by Adam Osuch » Sun, 27 May 2007 02:50:11



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 ...
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[Bridge] [BUG] Dropping fragmented IP packets within VLAN frames on bridge

Post by Ingo Oese » Sun, 27 May 2007 23:30:08


And could we separe the conditions for that into a static helper function
explaining each of these conditions? e.g. sth. like that:

static bool br_nf_need_fragment(struct sk_buff *skb)
{
/* Plain IP packet does not fit in MTU */
if (!(skb->protocol == htons(ETH_P_IP) && skb->len > skb->dev->mtu))
return true;

/* VLAN encapsulated IP packet does not fit in MTU */
if (IS_VLAN_IP(skb) && skb->len > skb->dev->mtu - VLAN_HLEN)
return true;

/* PPPoE encapsulated IP packet does not fit in MTU */
if (IS_PPPOE_IP(skb) && skb->len > skb->dev->mtu - PPPOE_SES_HLEN)
return true;

return false;
}

and then br_nf_dev_queue_xmit() becomes:

static int br_nf_dev_queue_xmit(struct sk_buff *skb)
{
if (br_nf_need_fragment(skb) && !skb_is_gso(skb))
return ip_fragment(skb, br_dev_queue_push_xmit);
else
return br_dev_queue_push_xmit(skb);
}

which is much more readable, more documented and doesn't contain a condition monster :-)

@Patrick: Could you check, wether the PPPoE case is correct?

What do you think? Should I submit a patch for that?


Best Regards

Ingo Oeser
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/