[ovs-dev] [PATCH 2/2] datapath: Allow minimum headroom to be set when copying buffers.
Ben Pfaff
blp at nicira.com
Wed Nov 18 21:22:50 UTC 2009
Jesse Gross <jesse at nicira.com> writes:
> If we need to copy an sk_buff in order to make it writable, allow
> the minimum headroom to be specified. This ensures that if we
> need to add additional data, such as a VLAN tag, we will not have
> to make a second copy.
Good catch.
> if (skb_shared(skb) || skb_cloned(skb)) {
> - struct sk_buff *nskb = skb_copy(skb, gfp);
> + struct sk_buff *nskb;
> + int headroom = min_headroom > skb_headroom(skb) ? min_headroom
> + : skb_headroom(skb);
I would write this as
unsigned headroom = max(min_headroom, skb_headroom(skb));
and change "min_headroom" to an unsigned int.
(Outside the Linux kernel this might not be a good idea. But the
Linux kernel definition of "max" only evaluates its arguments
once.)
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,24)
> + /* Before 2.6.24 these fields were not copied when
> + * doing an skb_copy_expand. */
> + nskb->ip_summed = skb->ip_summed;
> + nskb->csum = skb->csum;
> +#endif
Wow, that's real attention to detail, great.
> diff --git a/datapath/actions.h b/datapath/actions.h
> index bda6d55..f6eec29 100644
> --- a/datapath/actions.h
> +++ b/datapath/actions.h
> @@ -16,7 +16,7 @@ struct sk_buff;
> struct odp_flow_key;
> union odp_action;
>
> -struct sk_buff *make_writable(struct sk_buff *, gfp_t gfp);
> +struct sk_buff *make_writable(struct sk_buff *, int min_headroom, gfp_t gfp);
> int dp_xmit_skb(struct sk_buff *);
> int execute_actions(struct datapath *dp, struct sk_buff *skb,
> struct odp_flow_key *key,
I can't see any users of this function outside actions.c, could
we just make it static?
More information about the dev
mailing list