[ovs-dev] [PATCH v5] gre: Restructure tunneling.

[Jesse Gross]

On Wed, Jun 12, 2013 at 5:46 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
> diff --git a/datapath/linux/compat/gre.c b/datapath/linux/compat/gre.c
> new file mode 100644
> index 0000000..fbb9fb9
> --- /dev/null
> +++ b/datapath/linux/compat/gre.c
> +struct sk_buff *gre_handle_offloads(struct sk_buff *skb, bool gre_csum)
> +{
> +       int err;
> +
> +       skb_reset_inner_headers(skb);
> +
> +       if (skb_is_gso(skb)) {
> +               if (gre_csum)
> +                       OVS_GSO_CB(skb)->fix_segment = gre_csum_fix;
> +       } else {
> +               if (skb->ip_summed == CHECKSUM_PARTIAL && gre_csum) {
> +                       err = skb_checksum_help(skb);
> +                       if (err)
> +                               goto error;
> +
> +               } else if (skb->ip_summed != CHECKSUM_PARTIAL)
> +                       skb->ip_summed = CHECKSUM_NONE;
> +       }
> +       return skb;
> +error:
> +       kfree_skb(skb);
> +       return ERR_PTR(err);
> +}
[...]
> +void gre_build_header(struct sk_buff *skb, const struct tnl_ptk_info *tpi,
> +                     int hdr_len)
> +{
> +       struct gre_base_hdr *greh;
> +
> +       __skb_push(skb, hdr_len);
> +
> +       greh = (struct gre_base_hdr *)skb->data;
> +       greh->flags = tnl_flags_to_gre_flags(tpi->flags);
> +       greh->protocol = tpi->proto;
> +
> +       if (tpi->flags & (TUNNEL_KEY | TUNNEL_CSUM | TUNNEL_SEQ)) {
> +               __be32 *ptr = (__be32 *)(((u8 *)greh) + hdr_len - 4);
> +
> +               if (tpi->flags & TUNNEL_SEQ) {
> +                       *ptr = tpi->seq;
> +                       ptr--;
> +               }
> +               if (tpi->flags & TUNNEL_KEY) {
> +                       *ptr = tpi->key;
> +                       ptr--;
> +               }
> +               if (tpi->flags & TUNNEL_CSUM && !is_gre_gso(skb)) {
> +                       *ptr = 0;
> +                       *(__sum16 *)ptr = csum_fold(skb_checksum(skb, 0,
> +                                               skb->len, 0));
> +               }
> +       }
> +}

Are these two checksum calls safe in the presence of shared frags?

> diff --git a/datapath/linux/compat/gso.c b/datapath/linux/compat/gso.c
> new file mode 100644
> index 0000000..8cb2e06
> --- /dev/null
> +++ b/datapath/linux/compat/gso.c
> +int rpl_ip_local_out(struct sk_buff *skb)
> +{
[...]
> +       }  else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +               int err;
> +
> +               /* Use default features for dst device. */
> +               if (unlikely(skb_needs_linearize(skb, dst->dev->features))) {
> +                       err = __skb_linearize(skb);
> +                       if (unlikely(err))
> +                               return 0;
> +               }
> +
> +               err = skb_checksum_help(skb);
> +               if (unlikely(err))
> +                       return 0;

If we have the new version of skb_checksum_help (the one that checks
if a frag is shared) then I don't think we need to force linearization
and if we have the old one then I think we always need to linearize
regardless of the device features.

> +               id = -1;
> +       }
> +
> +       while (skb) {
> +               struct sk_buff *next_skb = skb->next;
> +               struct iphdr *iph;
> +               int err;
> +
> +               if (next_skb)
> +                       dst_clone(dst);

Doesn't skb_gso_segment() increment the refcount when it adds the dst
to each new segment?