Love, Robert W wrote:
> Joe Eykholt wrote:
>> 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.
>>
> You're right! I'm getting traffic on the correct queue too.
> do you get a segfault when you try to show the filter though?
> I haven't debugged it. Would you object to me checking the
> current patch in and then optimizing it once we can get to the
> bottom of the segfault?
I don't see the segfault. But I got duplicates due to a goof up on
my part, and then I can't delete them. I find the tc command is
very hard to use. Maybe my duplicates avoid the segfault?
> I'd prefer to not optimize the filter until we can show it as
> well, for debugging purposes. I'll do some debugging regardless,
> but I'd rather not hold up the FIP filtering on this detail if
> it's going to cause me to do a deep dive that could take some time.
That's fine with me.
> [r...@itchy ~]# tc filter show dev eth3
> filter parent 8001: protocol [35078] pref 49151 basic
> filter parent 8001: protocol [35078] pref 49151 basic handle 0x3003039
> Segmentation fault
Regards,
Joe
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
