Hi Yipeng,
yeah, this case should be handled as well - note that the ACL lookup
hash is 48x8, while the session lookup hash is 40x8, and the
fa_5tuple_t (being 48 bytes in size) contains the fa_packet_info_t
which is sitting at the last 8 bytes.
That data structure has a bunch of flag-type information derived from
the packet:
typedef union {
u64 as_u64;
struct {
u32 sw_if_index;
u16 mask_type_index_lsb;
u8 tcp_flags;
u8 tcp_flags_valid:1;
u8 is_input:1;
u8 l4_valid:1;
u8 is_nonfirst_fragment:1;
u8 is_ip6:1;
u8 flags_reserved:3;
};
} fa_packet_info_t;
They are filled during the initial extraction of the 5-tuple, and are
also used for the lookup in the hash table (this way we can match
non-initial fragments differently from the initial ones).
Note the "mask_type_index_lsb" field - when preparing for the lookup
iteration in multi_acl_match_get_applied_ace_index, it is filled with
the index of the mask used:
kv_key->pkt.mask_type_index_lsb = mask_type_index;
So in the example you cite, there will be two lookups in
multi_acl_match_get_applied_ace_index, which will both match, but the
one which is earlier in the ACLs will win, because of the check in "if
(result_val->applied_entry_index < curr_match_index)" - thus the end
result will be the same as if we just scanned the entire list
downwards.
Hopefully this clarifies the logic! :-)
--a
On 8/29/17, Wang, Yipeng1 <yipeng1.w...@intel.com> wrote:
> Thank you Andrew, seems I was looking at an older version...
>
> I was thinking of similar thing like using tuple space search like you
> already done. It is very good work and lots of considerations. One quick
> question:
>
> I just start looking at the code, but it seems all the rules are stored in
> one hash table (acl_lookup_hash). If a packet say having src IP= 1.1.0.0,
> dst=2.2.2.2, and there are two rules one is src=1.1.0.0/16, dst=2.2.0.0/32
> and the other is src=1.1.0.0/32, dst=2.2.0.0/16. The two rules will have
> same key value in the table right? When do lookup, assume packet header is
> masked by the second rule’s mask type and become src=1.1.0.0 dst=2.2.0.0.
> Will it match the first rule as well by mistake?
>
> Thank you.
> Yipeng
>
> From: Andrew Yourtchenko [mailto:ayour...@gmail.com]
> Sent: Sunday, August 27, 2017 6:30 AM
> To: Wang, Yipeng1 <yipeng1.w...@intel.com>
> Cc: vpp-dev@lists.fd.io; zhang...@yunshan.net.cn
> Subject: Re: [vpp-dev] ACL Match in fa_node.c
>
> Hi Yipeng,
>
> It's already there - just have a look through hash_* files in the ACL plugin
> directory on the master or latest stable/1707 :-)
>
> There are several things more that can be taken care of (e.g. the
> determination of the "ACE not shadowed" shortcut for applied ACEs would
> allow some good further improvement, or more distinction between the v4 and
> v6 lookups, and potentially more cleverness with handling of port ranges).
>
> Also, if you have ideas for a different lookup mechanism - feel free to add
> one - using a similar API as for the hash-based one it should be fairly
> straightforward.
>
> Do you have some specific ideas in mind ?
>
> --a
>
> On 26 Aug 2017, at 03:23, Wang, Yipeng1
> <yipeng1.w...@intel.com<mailto:yipeng1.w...@intel.com>> wrote:
> Hi, Andrew and Pan,
>
> I came across the thread you are talking about improvements of ACL plugin.
> Any update? I am also interested in the performance of ACL, wondering what
> is the direction of the optimizations you are working on. Could you share?
>
> Thanks
> Yipeng
>
>
>
>
>>On 5/23/17, å¼ æ”€ <zhangpan at
>> yunshan.net.cn<https://lists.fd.io/mailman/listinfo/vpp-dev>> wrote:
>
>> Hi Andrew!
>
>>
>
>>
>
>> ------------------ Original ------------------
>
>> From: "Andrew  Yourtchenko"<ayourtch at
>> gmail.com<https://lists.fd.io/mailman/listinfo/vpp-dev>>;
>
>> Date: Tue, May 23, 2017 07:56 PM
>
>> To: "å¼ æ”€"<zhangpan at
>> yunshan.net.cn<https://lists.fd.io/mailman/listinfo/vpp-dev>>;
>
>> Cc: "vpp-dev"<vpp-dev at
>> lists.fd.io<https://lists.fd.io/mailman/listinfo/vpp-dev>>;
>
>> Subject: Re: [vpp-dev] ACL Match in fa_node.c
>
>>
>
>>
>
>> Hi!
>
>>
>
>> On 5/23/17, å¼ æ”€ <zhangpan at
>> yunshan.net.cn<https://lists.fd.io/mailman/listinfo/vpp-dev>> wrote:
>
>>> Hi guys,
>
>>>
>
>>>
>
>>> I looked into the source code of vpp/src/plugin/acl/fa_node.c,
>
>>> in function full_acl_match_5tuple(), it seems that every ingress packet
>
>>> is
>
>>> matching against each ACL rule stored in acl_main->acls in a for-loop
>
>>> manner. This seems not fairly effective.
>
>>
>
>> You're absolutely right on both counts. First make it work, then make
>
>> it right, then make it fast :-) I have some ideas that I wanted to
>
>> experiment with there, would you be interested to help ? ACL matching
>
>> is a fairly distinct function operation to not affect much else.
>
>>
>
>> [PAN]: I would be very pleased to help as I am also a addicted person to
>
>> high performance programming :D
>
>>
>
>>
>
>
>
> Great! :-) I will try to sketch the idea tomorrow/thursday and will
>
> add you to the draft, so we can work together. (I also will have
>
> limited connectivity the next week, so I won't be a lot in your way!
>
> :-) (or if you have some good idea on how you'd like to do it, feel
>
> free to shoot a doc/code into a gerrit draft, let's see!)
>
>
>
>>
>
>>>
>
>>>
>
>>> Besides, I notice that in vpp/src/plugin/acl/acl.c，when you call the
>
>>> function acl_hook_l2_input_classify(), you will create a
>
>>> vnet_classify_table, but I didn't see any code which adds
>
>>> classify_session
>
>>> to it, why?
>
>>
>
>> I had used classify table for storing the sessions in the pre-1704
>
>> version of the ACL plugin.
>
>>
>
>> in 1704 as I was adding the L3, I moved to the new data path while
>
>> keeping the old one still around, and potentially switchable (not
>
>> terribly gracefully, but still).
>
>>
>
>> In the current master the classifier is used merely as a hook to get
>
>> into the packet processing within the L2 packet path - that's why you
>
>> see no sessions added.
>
>>
>
>> [PAN]: Cool. Correct me if I am wrong: ingress packet will first check
>
>> against if there is matched session existing
>
>> in fa_node.c and if not, the packet will check against ACL then to decide
>
>> whether to create a new session or drop.
>
>
>
> Exactly, that is the idea!
>
>
>
>>
>
>>
>
>> Is there any 'match then action' alike behavior existing in fa_node? And
>> a
>
>> long standing mystery question for me: what
>
>> does "fa" stand for :p
>
>>
>
>
>
> "FA" is "feature arc" :) When adding the routed path, the feature arc
>
> support appeared a bit earlier, and I was "wow, this is so excellent
>
> and easy", and I thought I might use the same mechanism in L2 as well,
>
> so I just bluntly called everything "fa_*". But then I decided to
>
> leave for L2 data path the old mechanism of (ab)using the L2
>
> classifier, since it allowed for a while to retain (just in case) the
>
> ability to switch between the old and the new data path. As the time
>
> goes, I suspect there might be a better way to hook in L2 forwarding
>
> path...
>
>
>
>>
>
>>
>
>>>
>
>>>
>
>>> Is there any document/idea could basically explain the relationships
>
>>> between
>
>>> acl/fa_node and vnet_classify?
>
>>
>
>> In a gerrit draft that I am working now on, which aims to provide the
>
>> multicore support, there is text file with the history and the
>
>> description of operation. If you like, I can add you to the reviewers
>
>> list so you can have a look at that - just send me your email that you
>
>> use in gerrit.
>
>>
>
>> [PAN]: Thank you Andrew, I believe this will help a lot. My email is
>
>> dazhangpan at gmail.com<https://lists.fd.io/mailman/listinfo/vpp-dev>
>
>>
>
>
>
> Excellent, I've just added you, it's 6771. As you notice, it's still a
>
> good amount of work to do (lots of clib_warning()s to ensure
>
> everything indeed does what I think it does). Looking forward to your
>
> feedback! :-)
>
>
>
> --a
>
>
>
>>
>
>>
>
>> --a
>
>>
>
>>>
>
>>>
>
>>> Any help will be much appreciated.
>
>>>
>
>>>
>
>>> Best Regards,
>
>>>
>
>>>
>
>>> Pan
>
>
_______________________________________________
vpp-dev mailing list
vpp-dev@lists.fd.io
https://lists.fd.io/mailman/listinfo/vpp-dev
