[ovs-dev] [PATCH] datapath: make skb->csum consistent with rest of networking stack.

[Pravin Shelar]

On Mon, Jun 10, 2013 at 5:23 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Mon, Jun 10, 2013 at 3:56 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
>> diff --git a/datapath/actions.c b/datapath/actions.c
>> index 0dac658..6d60cd0 100644
>> --- a/datapath/actions.c
>> +++ b/datapath/actions.c
>> @@ -132,9 +132,17 @@ static int set_eth_addr(struct sk_buff *skb,
>>         if (unlikely(err))
>>                 return err;
>>
>> +       if (get_ip_summed(skb) == OVS_CSUM_COMPLETE)
>> +               skb->csum = csum_sub(skb->csum, csum_partial(eth_hdr(skb),
>> +                                                            ETH_HLEN, 0));
>> +
>>         memcpy(eth_hdr(skb)->h_source, eth_key->eth_src, ETH_ALEN);
>>         memcpy(eth_hdr(skb)->h_dest, eth_key->eth_dst, ETH_ALEN);
>>
>> +       if (get_ip_summed(skb) == OVS_CSUM_COMPLETE)
>> +               skb->csum = csum_add(skb->csum, csum_partial(eth_hdr(skb),
>> +                                                            ETH_HLEN, 0));
>> +
>
> The add and subtract operations can be combined into a single
> operation - see csum_replace4() for an example.
>
I tried to do this, But doing it will involve one copy operation (or
two if I write csum_replace_thehdr function) due to eth_key and ethhdr
struct differences.
so I am not sure abt it.

>> diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
>> index 9a159cd..4fdbc59 100644
>> --- a/datapath/vport-internal_dev.c
>> +++ b/datapath/vport-internal_dev.c
>> @@ -267,6 +267,7 @@ static int internal_dev_recv(struct vport *vport, struct sk_buff *skb)
>>         struct net_device *netdev = netdev_vport_priv(vport)->dev;
>>         int len;
>>
>> +       forward_ip_summed(skb, false);
>>  #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,37)
>>         if (unlikely(vlan_deaccel_tag(skb)))
>>                 return 0;
>
> I think we also need to handle this in vlan_deaccel_tag() as well.
>
ok.

> In LISP, even though there is no Ethernet header to start out with we
> add one later, so it needs the same treatment.

ok.