is CONFIG_NET_CLS_ACT always set ?

is CONFIG_NET_CLS_ACT always set ?

[Laurent Chavey]

if CONFIG_NET_CLS_ACT is not defined, then is the
code below correct ?


netif_receive_skb()
---------------------------
	list_for_each_entry_rcu(ptype, &ptype_all, list) {
		if (!ptype->dev || ptype->dev == skb->dev) {
			if (pt_prev)
				ret = deliver_skb(skb, pt_prev);
			pt_prev = ptype;
		}
	}

#ifdef CONFIG_NET_CLS_ACT
	if (pt_prev) {
		ret = deliver_skb(skb, pt_prev);
		pt_prev = NULL; /* noone else should process this after*/
	} else {
		skb->tc_verd = SET_TC_OK2MUNGE(skb->tc_verd);
	}

	ret = ing_filter(skb);

	if (ret == TC_ACT_SHOT || (ret == TC_ACT_STOLEN)) {
		kfree_skb(skb);
		goto out;
	}

	skb->tc_verd = 0;
ncls:
#endif
handle_diverter(skb);

if (handle_bridge(&skb, &pt_prev, &ret))
	goto out;

Re: is CONFIG_NET_CLS_ACT always set ?

[jamal]

On Mon, 2007-23-04 at 14:19 -0700, Laurent Chavey wrote:
> if CONFIG_NET_CLS_ACT is not defined, then is the
> code below correct ?

The simple answer is yes; 
Is something not working for you?

It is hard to read your question my good friend because it comes out
like a trick question;-> 
It is more useful to say:
 "I think this code is broken because ...."
And even more useful if you add:
"I have run the following testcases to illustrate blah ..."
Then if something is broken, and you want to be popular, you say:
"netdevers, heres the fix"

Anyways, what is the problem?

cheers,
jamal

Re: is CONFIG_NET_CLS_ACT always set ?

[Laurent Chavey]

sorry for the trick question.

I was simply making a modification to the skb tap code
to allow tcpdump to work on the slave interface and not
just the bonded one. The code change requires a change to
the for loop code and it follow on to the call to deliver_skb().

In the process of adding the code, I noticed that if (at least
that is how I read the code)
CONFIG_NET_CLS_ACT is not defined then
the last pt_prev is call after the diverter and bridge code
have handled the skb.  This would only occur for bridged skb
that one wanted to tap.



On 4/23/07, jamal <hadi@cyberus.ca> wrote:
> On Mon, 2007-23-04 at 14:19 -0700, Laurent Chavey wrote:
> > if CONFIG_NET_CLS_ACT is not defined, then is the
> > code below correct ?
>
> The simple answer is yes;
> Is something not working for you?
>
> It is hard to read your question my good friend because it comes out
> like a trick question;->
> It is more useful to say:
>  "I think this code is broken because ...."
> And even more useful if you add:
> "I have run the following testcases to illustrate blah ..."
> Then if something is broken, and you want to be popular, you say:
> "netdevers, heres the fix"
>
> Anyways, what is the problem?
>
> cheers,
> jamal
>
>

Re: is CONFIG_NET_CLS_ACT always set ?

[jamal]

On Mon, 2007-23-04 at 16:08 -0700, Laurent Chavey wrote:
> sorry for the trick question.
> 
> I was simply making a modification to the skb tap code
> to allow tcpdump to work on the slave interface and not
> just the bonded one. 

If you dont mind: what is the goal? I am not sure i want to see the
packets on a backup interface in a bond active/backup - but you may
have some good reason.

> The code change requires a change to
> the for loop code and it follow on to the call to deliver_skb().
> 

Iam assuming you have taken care of the code above that which drops the
packet if not destined for master? 

> In the process of adding the code, I noticed that if (at least
> that is how I read the code)
> CONFIG_NET_CLS_ACT is not defined then
> the last pt_prev is call after the diverter and bridge code
> have handled the skb.  This would only occur for bridged skb
> that one wanted to tap.
> 

The bridge code does the right thing if you follow handle bridge.
The old diverter (you must be looking at an old kernel, because it is
gone) was supposed to be able to steal packets.
I am not sure if that answers your question.

cheers,
jamal

Re: is CONFIG_NET_CLS_ACT always set ?

[Laurent Chavey]

On 4/23/07, jamal <hadi@cyberus.ca> wrote:
> On Mon, 2007-23-04 at 16:08 -0700, Laurent Chavey wrote:
> > sorry for the trick question.
> >
> > I was simply making a modification to the skb tap code
> > to allow tcpdump to work on the slave interface and not
> > just the bonded one.
>
> If you dont mind: what is the goal? I am not sure i want to see the
> packets on a backup interface in a bond active/backup - but you may
> have some good reason.
this is for an lacp type bond where packets can come on
any of the bond links and we need to check for the exact link.

>
> > The code change requires a change to
> > the for loop code and it follow on to the call to deliver_skb().
> >
>
> Iam assuming you have taken care of the code above that which drops the
> packet if not destined for master?
yes
>
> > In the process of adding the code, I noticed that if (at least
> > that is how I read the code)
> > CONFIG_NET_CLS_ACT is not defined then
> > the last pt_prev is call after the diverter and bridge code
> > have handled the skb.  This would only occur for bridged skb
> > that one wanted to tap.
> >
>
> The bridge code does the right thing if you follow handle bridge.
> The old diverter (you must be looking at an old kernel, because it is
> gone) was supposed to be able to steal packets.
> I am not sure if that answers your question.
>
given 2 skb, both bridged (stolen) by the bridge code,
if CONFIG_NET_CLS_ACT is not defined, then it looks like that
only the first skb would be taped, the second one never makes
it to the tap.


> cheers,
> jamal
>
>
>

Re: is CONFIG_NET_CLS_ACT always set ?

[Laurent Chavey]

handle_bridge does call the tap, so my question is answer there.
thanks,

On 4/24/07, Laurent Chavey <chavey@google.com> wrote:
> On 4/23/07, jamal <hadi@cyberus.ca> wrote:
> > On Mon, 2007-23-04 at 16:08 -0700, Laurent Chavey wrote:
> > > sorry for the trick question.
> > >
> > > I was simply making a modification to the skb tap code
> > > to allow tcpdump to work on the slave interface and not
> > > just the bonded one.
> >
> > If you dont mind: what is the goal? I am not sure i want to see the
> > packets on a backup interface in a bond active/backup - but you may
> > have some good reason.
> this is for an lacp type bond where packets can come on
> any of the bond links and we need to check for the exact link.
>
> >
> > > The code change requires a change to
> > > the for loop code and it follow on to the call to deliver_skb().
> > >
> >
> > Iam assuming you have taken care of the code above that which drops the
> > packet if not destined for master?
> yes
> >
> > > In the process of adding the code, I noticed that if (at least
> > > that is how I read the code)
> > > CONFIG_NET_CLS_ACT is not defined then
> > > the last pt_prev is call after the diverter and bridge code
> > > have handled the skb.  This would only occur for bridged skb
> > > that one wanted to tap.
> > >
> >
> > The bridge code does the right thing if you follow handle bridge.
> > The old diverter (you must be looking at an old kernel, because it is
> > gone) was supposed to be able to steal packets.
> > I am not sure if that answers your question.
> >
> given 2 skb, both bridged (stolen) by the bridge code,
> if CONFIG_NET_CLS_ACT is not defined, then it looks like that
> only the first skb would be taped, the second one never makes
> it to the tap.
>
>
> > cheers,
> > jamal
> >
> >
> >
>