[PATCH 1/2] macvlan : fix checksums error when we are in bridge mode

[PATCH 1/2] macvlan : fix checksums error when we are in bridge mode

[Daniel Lezcano]

When the lower device has offloading capabilities, the packets checksums
are not computed. That leads to have any macvlan port in bridge mode to
not work because the packets are dropped due to a bad checksum.

If the macvlan is in bridge mode, the packet is forwarded to another
macvlan port and reach the network stack where it looks for a checksum
but this one was not computed due to the offloading of the lower device.
In this case, we have to set the packet with CHECKSUM_UNNECESSARY
when it is forwarded to a bridged port and restore the previous value of
ip_summed when the packet goes to the lowerdev.

Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Andrian Nord <nightnord@gmail.com>
---
 drivers/net/macvlan.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 6ed577b..497991b 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -219,9 +219,11 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
 	const struct macvlan_dev *vlan = netdev_priv(dev);
 	const struct macvlan_port *port = vlan->port;
 	const struct macvlan_dev *dest;
+	__u8 ip_summed = skb->ip_summed;
 
 	if (vlan->mode == MACVLAN_MODE_BRIDGE) {
 		const struct ethhdr *eth = (void *)skb->data;
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 		/* send to other bridge ports directly */
 		if (is_multicast_ether_addr(eth->h_dest)) {
@@ -241,6 +243,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 xmit_world:
+	skb->ip_summed = ip_summed;
 	skb_set_dev(skb, vlan->lowerdev);
 	return dev_queue_xmit(skb);
 }
-- 
1.7.1

[PATCH 2/2] dev : fix mtu check when TSO is enabled

[Daniel Lezcano]

In case the device where is coming from the packet has TSO enabled,
we should not check the mtu size value as this one could be bigger
than the expected value.

This is the case for the macvlan driver when the lower device has
TSO enabled. The macvlan inherit this feature and forward the packets
without fragmenting them. Then the packets go through dev_forward_skb
and are dropped. This patch fix this by checking TSO is not enabled
when we want to check the mtu size.

Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Andrian Nord <nightnord@gmail.com>
---
 net/core/dev.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 6561021..010a0a9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1526,8 +1526,10 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
 	skb_orphan(skb);
 	nf_reset(skb);
 
-	if (unlikely(!(dev->flags & IFF_UP) ||
-		     (skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
+	if (unlikely(!(skb->dev->features & NETIF_F_TSO) &&
+		     (!(dev->flags & IFF_UP) ||
+		      (skb->len > (dev->mtu +
+				   dev->hard_header_len + VLAN_HLEN))))) {
 		atomic_long_inc(&dev->rx_dropped);
 		kfree_skb(skb);
 		return NET_RX_DROP;
-- 
1.7.1

Re: [PATCH 2/2] dev : fix mtu check when TSO is enabled

[Eric Dumazet]

Le lundi 14 mars 2011 à 17:08 +0100, Daniel Lezcano a écrit :
> In case the device where is coming from the packet has TSO enabled,
> we should not check the mtu size value as this one could be bigger
> than the expected value.
> 
> This is the case for the macvlan driver when the lower device has
> TSO enabled. The macvlan inherit this feature and forward the packets
> without fragmenting them. Then the packets go through dev_forward_skb
> and are dropped. This patch fix this by checking TSO is not enabled
> when we want to check the mtu size.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Andrian Nord <nightnord@gmail.com>
> ---
>  net/core/dev.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6561021..010a0a9 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1526,8 +1526,10 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
>  	skb_orphan(skb);
>  	nf_reset(skb);
>  
> -	if (unlikely(!(dev->flags & IFF_UP) ||
> -		     (skb->len > (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
> +	if (unlikely(!(skb->dev->features & NETIF_F_TSO) &&


Sorry, this needs a comment at least, or even better a helper function
on its own.

We test both skb->dev & dev, pointing to different devices, this is
really error prone.

Are we sure all callers set skb->dev before calling  dev_forward_skb() ?


> +		     (!(dev->flags & IFF_UP) ||
> +		      (skb->len > (dev->mtu +
> +				   dev->hard_header_len + VLAN_HLEN))))) {
>  		atomic_long_inc(&dev->rx_dropped);
>  		kfree_skb(skb);
>  		return NET_RX_DROP;

Re: [PATCH 2/2] dev : fix mtu check when TSO is enabled

[Daniel Lezcano]

On 03/14/2011 06:01 PM, Eric Dumazet wrote:
> Le lundi 14 mars 2011 à 17:08 +0100, Daniel Lezcano a écrit :
>> In case the device where is coming from the packet has TSO enabled,
>> we should not check the mtu size value as this one could be bigger
>> than the expected value.
>>
>> This is the case for the macvlan driver when the lower device has
>> TSO enabled. The macvlan inherit this feature and forward the packets
>> without fragmenting them. Then the packets go through dev_forward_skb
>> and are dropped. This patch fix this by checking TSO is not enabled
>> when we want to check the mtu size.
>>
>> Signed-off-by: Daniel Lezcano<daniel.lezcano@free.fr>
>> Cc: Patrick McHardy<kaber@trash.net>
>> Cc: Andrian Nord<nightnord@gmail.com>
>> ---
>>   net/core/dev.c |    6 ++++--
>>   1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 6561021..010a0a9 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1526,8 +1526,10 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
>>   	skb_orphan(skb);
>>   	nf_reset(skb);
>>
>> -	if (unlikely(!(dev->flags&  IFF_UP) ||
>> -		     (skb->len>  (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
>> +	if (unlikely(!(skb->dev->features&  NETIF_F_TSO)&&
>
> Sorry, this needs a comment at least, or even better a helper function
> on its own.
>
> We test both skb->dev&  dev, pointing to different devices, this is
> really error prone.
>
> Are we sure all callers set skb->dev before calling  dev_forward_skb() ?

Thanks Eric for the comments, I will resend a new version.

   -- Daniel

Re: [PATCH 1/2] macvlan : fix checksums error when we are in bridge mode

[Eric Dumazet]

Le lundi 14 mars 2011 à 17:08 +0100, Daniel Lezcano a écrit :
> When the lower device has offloading capabilities, the packets checksums
> are not computed. That leads to have any macvlan port in bridge mode to
> not work because the packets are dropped due to a bad checksum.
> 
> If the macvlan is in bridge mode, the packet is forwarded to another
> macvlan port and reach the network stack where it looks for a checksum
> but this one was not computed due to the offloading of the lower device.
> In this case, we have to set the packet with CHECKSUM_UNNECESSARY
> when it is forwarded to a bridged port and restore the previous value of
> ip_summed when the packet goes to the lowerdev.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
> Cc: Patrick McHardy <kaber@trash.net>
> Cc: Andrian Nord <nightnord@gmail.com>

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>