Re: [PATCH] atm: introduce vcc_pop()

[Krzysztof Mazur <krzysiek@podlesie.net>]

On Wed, Nov 28, 2012 at 04:59:06PM -0500, chas williams - CONTRACTOR wrote:
> On Wed, 28 Nov 2012 22:45:34 +0100
> Krzysztof Mazur <krzysiek@podlesie.net> wrote:
> 
> > On Wed, Nov 28, 2012 at 04:20:01PM -0500, chas williams - CONTRACTOR wrote:
> > > i dont like the vcc->pop() implementation and at one point i had the
> > > crazy idea of using skb->destructors to handle it.  however, i think it
> > > would be necessary to clone the skb's so any existing destructor is
> > > preserved.
> > 
> > With this patch we will kill vcc->pop() in drivers and in future
> > we can do that without changes in drivers.
> 
> ok
> 
> > > 
> > > > +#define vcc_pop(vcc, skb) vcc_pop_any(vcc, skb)
> > > > +#define vcc_pop_irq(vcc, skb) vcc_pop_any(vcc, skb)
> > > 
> > > don't define these if you dont plan on using them anway.
> > 
> > I removed them. I also added check if vcc is NULL, as David Woodhouse
> > suggested, some drivers use that.
> 
> it should probably be if (likely(vcc) && likely(vcc->pop)) since it
> will almost always be the case.

Thanks,

Krzysiek
-- >8 --
Subject: [PATCH v3] atm: introduce vcc_pop()

The atm drivers to free skb, that they got from ->send(), cannot just use
dev_kfree_skb*(), but they must use something like:

	if (vcc->pop)
		vcc->pop(vcc, skb);
	else
		dev_kfree_skb_any(skb);

When vcc->pop() is non-NULL, but they must in such case call vcc->pop().
This causes duplicated code in many drivers, and some drivers even forgot
to call vcc->pop() in some error handling code.

Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
 include/linux/atmdev.h | 8 ++++++++
 net/atm/common.c       | 9 +++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/linux/atmdev.h b/include/linux/atmdev.h
index c1da539..57bd93f 100644
--- a/include/linux/atmdev.h
+++ b/include/linux/atmdev.h
@@ -283,6 +283,14 @@ int atm_pcr_goal(const struct atm_trafprm *tp);
 
 void vcc_release_async(struct atm_vcc *vcc, int reply);
 
+/**
+ * vcc_pop - free transmitted ATM skb
+ *
+ * vcc_pop() should be used by ATM driver to free skbs, that were sent
+ * to driver by atmdev_opt->send() function.
+ */
+void vcc_pop(struct atm_vcc *vcc, struct sk_buff *skb);
+
 struct atm_ioctl {
 	struct module *owner;
 	/* A module reference is kept if appropriate over this call.
diff --git a/net/atm/common.c b/net/atm/common.c
index 806fc0a..c42ff62 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -654,6 +654,15 @@ out:
 	return error;
 }
 
+void vcc_pop(struct atm_vcc *vcc, struct sk_buff *skb)
+{
+	if (likely(vcc) && likely(vcc->pop))
+		vcc->pop(vcc, skb);
+	else
+		dev_kfree_skb_any(skb);
+}
+EXPORT_SYMBOL(vcc_pop);
+
 unsigned int vcc_poll(struct file *file, struct socket *sock, poll_table *wait)
 {
 	struct sock *sk = sock->sk;
-- 
1.8.0.411.g71a7da8