Robert Love wrote:
> On Mon, 2009-05-04 at 11:01 -0700, Joe Eykholt wrote:
>> Robert Love wrote:
>>> On Fri, 2009-05-01 at 11:27 -0700, Joe Eykholt wrote:
>>>> Love, Robert W wrote:
>>> <snip>
>>>
>>>>>> find_skbedit_filter()
>>>>>> {
>>>>>> ifname=$1
>>>>>> + ethertype=$2
>>>>>> + filter_id=$3
>>>>>>
>>>>>> found=`tc filter show dev $ifname | awk '
>>>>>> BEGIN {
>>>>>> @@ -120,11 +118,11 @@ find_skbedit_filter()
>>>>>> x3 = 0
>>>>>> queue = 8
>>>>>> }
>>>>>> - /^filter.*parent.*protocol '$FCOE_ETHERTYPE'.* handle
>>>>>> '$FILTER_ID'/
>>>>>> { + /^filter.*parent.*protocol \['$ethertype'\].* handle
>>>>>> '$filter_id'/ { if (x1 == 0 && x2 == 0 && x3 == 0)
>>>>>> x1 = 1
>>>>>> }
>>>>>> - /cmp.*u16 at 12 layer 1 mask 0xffff eq '$FCOE_ETHERTYPE'.*\)/ {
>>>>>> + /cmp.*u16 at 12 layer 1 mask 0xffff eq '$ethertype'.*\)/ {
>>>> Now since we're matching on the
>>>> protocol, I don't think we need to do the equivalent compare of the
>>>> "u16 at 12", which is also the ethertype. That was only necessary before
>>>> tc could take arbitrary ethertypes. Of course, this doesn't do any harm
>>>> as far as I can tell.
>>>>
>>> Are you suggesting that we can remove the cmp or that the cmp should be
>>> written more elegantly? Can you show me the preferred syntax? I've
>>> played around, but I can't get it right.
>> It seems to me the cmp can be removed, since the filter is already specifying
>> the ethertype. tc matches the protocol and then does the cmp
>> which matches again. It used to be that we specified the protocol as 802.3
>> and then the cmp with the ethertype was needed. But now, it isn't.
>>
> My understanding is that the protocol field is only used to identify
> which header to look into, before running the compare operation, so even
> if we specify FCoE we're just getting to the L2 header and we'd still
> need to have compare criteria to find the FCoE ethertype. This would
> also require tc to be aware of the FCoE ethertype, which I don't think
> it is.
>
> Is there a specific kernel patch(s) that has changed this behavior that
> you can point me to? If it has changed, I'd like to know, but I'm not
> aware that it has. Or, if you can you tell me what the correct syntax
> would be to accomplish what you're asking for it would help a lot.
>
> Right now I'm not convinced that we can simplify this.
The code that runs the classifiers is in
net/sched/sch_api.c in function tc_classify_compat():
where it goes through the filters matching only those that match the
protocol in skb->protocol or that say they apply to all protocols.
The patch that changed skb->protocol in fcoe_xmit() is what
allows this simplification.
The syntax I used is:
tc filter add dev $ifname protocol $FCOE_ETHERTYPE \
handle $FILTER_ID basic \
action skbedit queue_mapping $queue
I tested this using 128K writes, and ethtool shows
the data went out on queue 3.
Joe
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
