| Message ID | 412e6f7f0911160033h649b44b2o669196b8b14bf6fc@mail.gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2009-11-16 at 16:33 +0800, Changli Gao wrote: > act_mirred: cleanup > > 1. don't let go back using goto. > 2. don't call skb_act_clone() until it is necessary. > 3. one exit of the critical context. > > Signed-off-by: Changli Gao <xiaosuo@gmail.com> Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca> cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: jamal <hadi@cyberus.ca> Date: Mon, 16 Nov 2009 04:24:39 -0500 > On Mon, 2009-11-16 at 16:33 +0800, Changli Gao wrote: >> act_mirred: cleanup >> >> 1. don't let go back using goto. >> 2. don't call skb_act_clone() until it is necessary. >> 3. one exit of the critical context. >> >> Signed-off-by: Changli Gao <xiaosuo@gmail.com> > > Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 16, 2009 at 6:48 PM, David Miller <davem@davemloft.net> wrote: > From: jamal <hadi@cyberus.ca> > Date: Mon, 16 Nov 2009 04:24:39 -0500 > >> On Mon, 2009-11-16 at 16:33 +0800, Changli Gao wrote: >>> act_mirred: cleanup >>> >>> 1. don't let go back using goto. >>> 2. don't call skb_act_clone() until it is necessary. >>> 3. one exit of the critical context. >>> >>> Signed-off-by: Changli Gao <xiaosuo@gmail.com> >> >> Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca> > > Applied. > Patch is resubmitted as attachment.
From: Changli Gao <xiaosuo@gmail.com> Date: Tue, 17 Nov 2009 13:48:06 +0800 > Patch is resubmitted as attachment. That's a lot better, applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index b9aaab4..b812c20 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -148,47 +148,39 @@ static int tcf_mirred(struct sk_buff *skb, struct tc_action *a, { struct tcf_mirred *m = a->priv; struct net_device *dev; - struct sk_buff *skb2 = NULL; - u32 at = G_TC_AT(skb->tc_verd); + struct sk_buff *skb2; + u32 at; + int retval, err = 1; spin_lock(&m->tcf_lock); - - dev = m->tcfm_dev; m->tcf_tm.lastuse = jiffies; + if (m->tcfm_eaction != TCA_EGRESS_MIRROR && + m->tcfm_eaction != TCA_EGRESS_REDIR) { + if (net_ratelimit()) + printk("tcf_mirred unknown action %d\n", + m->tcfm_eaction); + goto out; + } - if (!(dev->flags&IFF_UP) ) { + dev = m->tcfm_dev; + if (!(dev->flags & IFF_UP)) { if (net_ratelimit()) printk("mirred to Houston: device %s is gone!\n", dev->name); -bad_mirred: - if (skb2 != NULL) - kfree_skb(skb2); - m->tcf_qstats.overlimits++; - m->tcf_bstats.bytes += qdisc_pkt_len(skb); - m->tcf_bstats.packets++; - spin_unlock(&m->tcf_lock); - /* should we be asking for packet to be dropped? - * may make sense for redirect case only - */ - return TC_ACT_SHOT; + goto out; } skb2 = skb_act_clone(skb, GFP_ATOMIC); if (skb2 == NULL) - goto bad_mirred; - if (m->tcfm_eaction != TCA_EGRESS_MIRROR && - m->tcfm_eaction != TCA_EGRESS_REDIR) { - if (net_ratelimit()) - printk("tcf_mirred unknown action %d\n", - m->tcfm_eaction); - goto bad_mirred; - } + goto out; m->tcf_bstats.bytes += qdisc_pkt_len(skb2); m->tcf_bstats.packets++; - if (!(at & AT_EGRESS)) + at = G_TC_AT(skb->tc_verd); + if (!(at & AT_EGRESS)) { if (m->tcfm_ok_push) skb_push(skb2, skb2->dev->hard_header_len); + } /* mirror is always swallowed */
act_mirred: cleanup 1. don't let go back using goto. 2. don't call skb_act_clone() until it is necessary. 3. one exit of the critical context. Signed-off-by: Changli Gao <xiaosuo@gmail.com> ---- net/sched/act_mirred.c | 59 +++++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 26 deletions(-) if (m->tcfm_eaction != TCA_EGRESS_MIRROR) @@ -197,8 +189,23 @@ bad_mirred: skb2->dev = dev; skb2->iif = skb->dev->ifindex; dev_queue_xmit(skb2); + err = 0; + +out: + if (err) { + m->tcf_qstats.overlimits++; + m->tcf_bstats.bytes += qdisc_pkt_len(skb); + m->tcf_bstats.packets++; + /* should we be asking for packet to be dropped? + * may make sense for redirect case only + */ + retval = TC_ACT_SHOT; + } else { + retval = m->tcf_action; + } spin_unlock(&m->tcf_lock); - return m->tcf_action; + + return retval; } static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind, int ref) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html