lkml.org 
[lkml]   [2012]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH] atm: introduce vcc_pop()
    On Wed, Nov 28, 2012 at 04:20:01PM -0500, chas williams - CONTRACTOR wrote:
    > On Wed, 28 Nov 2012 21:18:37 +0100
    > Krzysztof Mazur <krzysiek@podlesie.net> wrote:
    >
    > > On Tue, Nov 27, 2012 at 07:28:43PM +0100, Krzysztof Mazur wrote:
    > > > I think that we should add atm_pop() function that does that and fix all
    > > > drivers.
    > > >
    > >
    > > I'm sending a patch that implements that idea.
    > >
    > > Currently we need two arguments vcc and skb. However, we have reserved
    > > ATM_SKB(skb)->vcc in skb control block for keeping vcc
    > > and we can create single argument version vcc_pop(skb). In that case
    > > we need to move:
    > >
    > > ATM_SKB(skb)->vcc = vcc;
    > >
    > > from ATM drivers to functions that call atmdev_ops->send().
    >
    > 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.

    >
    > > +#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.

    Krzysiek
    -- >8 --
    Subject: [PATCH v2] 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..76bf76c 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 (vcc && 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


    \
     
     \ /
      Last update: 2012-11-28 23:21    [W:4.312 / U:0.508 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site