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'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.
[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
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
