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

Jesse Gross jesse at nicira.com
Tue Jun 11 22:30:59 UTC 2013


On Tue, Jun 11, 2013 at 1:20 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));
> +

I guess we could make this marginally more efficient by using 2 *
ETH_ALEN instead of ETH_HLEN since that's the part that is actually
changing.

> diff --git a/datapath/linux/compat/include/linux/skbuff.h b/datapath/linux/compat/include/linux/skbuff.h
> index d485b39..9bf7493 100644
> --- a/datapath/linux/compat/include/linux/skbuff.h
> +++ b/datapath/linux/compat/include/linux/skbuff.h
> +static inline void skb_postpush_rcsum(struct sk_buff *skb,
> +                                     const void *start, unsigned int len)
> +{
> +       if (skb->ip_summed == CHECKSUM_COMPLETE)
> +               skb->csum = csum_add(skb->csum, csum_partial(start, len, 0));
> +}

Do we think that this will be generally useful outside of OVS? I
suspect that there are very few, if any, other users in the network
stack that add data to packets on the receive side. If that's the case
then we might want to just keep in internal.

> 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;
> @@ -281,7 +282,7 @@ static int internal_dev_recv(struct vport *vport, struct sk_buff *skb)
>         skb->dev = netdev;
>         skb->pkt_type = PACKET_HOST;
>         skb->protocol = eth_type_trans(skb, netdev);
> -       forward_ip_summed(skb, false);
> +       skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
>
>         netif_rx(skb);

Since we're doing checksum manipulations, it's probably better to put
forward_ip_summed() at the end since it basically represents the end
of OVS's internal checksum normalization.



More information about the dev mailing list