Re: [PATCH net-next 2/8] skb: api to report errors for zero copy skbs

Michael S. Tsirkin
2012-10-30T15:52:28Z

On Tue, Oct 30, 2012 at 11:44:16AM -0400, Vlad Yasevich wrote:
> On 10/29/2012 11:49 AM, Michael S. Tsirkin wrote:
> >Orphaning frags for zero copy skbs needs to allocate data in atomic
> >context so is has a chance to fail. If it does we currently discard
> >the skb which is safe, but we don't report anything to the caller,
> >so it can not recover by e.g. disabling zero copy.
> >
> >Add an API to free skb reporting such errors: this is used
> >by tun in case orphaning frags fails.
> >
> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >---
> > include/linux/skbuff.h | 1 +
> > net/core/skbuff.c | 19 +++++++++++++++++++
> > 2 files changed, 20 insertions(+)
> >
> >diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> >index 8bac11b..0644432 100644
> >--- a/include/linux/skbuff.h
> >+++ b/include/linux/skbuff.h
> >@@ -568,6 +568,7 @@ static inline struct rtable *skb_rtable(const struct sk_buff *skb)
> > }
> >
> > extern void kfree_skb(struct sk_buff *skb);
> >+extern void skb_tx_error(struct sk_buff *skb, int err);
> > extern void consume_skb(struct sk_buff *skb);
> > extern void __kfree_skb(struct sk_buff *skb);
> > extern struct kmem_cache *skbuff_head_cache;
> >diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> >index eb31f6e..ad99c64 100644
> >--- a/net/core/skbuff.c
> >+++ b/net/core/skbuff.c
> >@@ -635,6 +635,25 @@ void kfree_skb(struct sk_buff *skb)
> > EXPORT_SYMBOL(kfree_skb);
> >
> > /**
> >+ * kfree_skb_on_error - report an sk_buff xmit error
> >+ * @skb: buffer that triggered an error
> >+ *
> >+ * Report xmit error if a device callback is tracking this skb.
> >+ */
>
> Nit: Comment doesn't match new function.
>
> -vlad

Good catch, thanks.

> >+void skb_tx_error(struct sk_buff *skb, int err)
> >+{
> >+ if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
> >+ struct ubuf_info *uarg;
> >+
> >+ uarg = skb_shinfo(skb)->destructor_arg;
> >+ if (uarg->callback)
> >+ uarg->callback(uarg, err);
> >+ skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
> >+ }
> >+}
> >+EXPORT_SYMBOL(skb_tx_error);
> >+
> >+/**
> > * consume_skb - free an skbuff
> > * @skb: buffer to free
> > *
> >