Hi Neale,
Thanks for the reply. I agree on 2). Tracking opaque’s validity is work I will have to look into. Cheers, Hemant From: Neale Ranns <ne...@graphiant.com> Sent: Wednesday, January 20, 2021 4:18 AM To: hem...@mnkcg.com; ayour...@gmail.com Cc: vpp-dev@lists.fd.io Subject: Re: [vpp-dev] classifier howto? Hi Hemant, I’ll be a little more direct 😊 you can’t expand the size of opaque. The blast radius of such a change is just too large. Your alternative choices are: 1) use oqaque2, 2) overlay your ‘value’ field in the union in opaque. The latter is preferable since you won’t incur the cost of loading the other cacheline, but you will need to be sure that the value remains valid (i.e. is not overwritten) by intermediate nodes from the time it is set to the time it is used. /neale From: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> <vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> > on behalf of hemant via lists.fd.io <hemant=mnkcg....@lists.fd.io <mailto:hemant=mnkcg....@lists.fd.io> > Date: Wednesday, 20 January 2021 at 02:51 To: ayour...@gmail.com <mailto:ayour...@gmail.com> <ayour...@gmail.com <mailto:ayour...@gmail.com> > Cc: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> <vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> > Subject: Re: [vpp-dev] classifier howto? Andrew, The good thing with the code changes are that the changes are common to any plugin or base code tests. Any CSIT testing could take the diffs and test for sanity. I also wonder when the opaque2 member (line of code below) was added to vlib_buffer_t the cache line got impacted by a factor larger than 2 to 3 cache lines of these changes. https://github.com/FDio/vpp/blob/5c1e48c01b50ddbd7623228e3dbc94d835d23813/src/vlib/buffer.h#L170 Anyone recall the perf changes with opaque2? For my traffic setup (uses trex and 10G NICs) and a UPF IPv4 test, I don’t see appreciable difference testing traffic with our without the code changes. Thanks, Hemant From: Andrew 👽 Yourtchenko <ayour...@gmail.com <mailto:ayour...@gmail.com> > Sent: Tuesday, January 19, 2021 6:21 PM To: hem...@mnkcg.com <mailto:hem...@mnkcg.com> Cc: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> Subject: Re: [vpp-dev] classifier howto? the further discussion to be held in 30844 with the corresponding SMEs, but in my cursory virw it’s tweaking the buffer metadata area which is quite erformance critical - and if I read well https://gerrit.fd.io/r/c/vpp/+/30844/2/src/vlib/buffer.h#194 increases the per-buffer metadata size from 2 to 3 cache lines... what is the performance impact of this change for all of the existing functions? (Might make sense to proactively include this analysis into 30844, since I’d expect it the very first question to be asked during the review). --a On 20 Jan 2021, at 00:06, hem...@mnkcg.com <mailto:hem...@mnkcg.com> wrote: Andrew, Sorry, I am used to github which auto-merges two commits and missed using gerrit ‘s –amend in my 2nd commit. I did what you unicasted to me and now the latest diffs are available in review. https://gerrit.fd.io/r/c/vpp/+/30848 thanks, Hemant From: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> <vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> > On Behalf Of Andrew Yourtchenko Sent: Tuesday, January 19, 2021 4:40 PM To: hem...@mnkcg.com <mailto:hem...@mnkcg.com> Cc: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> Subject: Re: [vpp-dev] classifier howto? On 19 Jan 2021, at 22:17, hem...@mnkcg.com <mailto:hem...@mnkcg.com> wrote: Andrew, Great suggestion – thanks. I updated the code after taking care of your comments. https://gerrit.fd.io/r/c/vpp/+/30848/1 Seems like you pushed two different changes - the better approach is to reuse the “Change-ID: xxxx” value from the previous edit by doing “git commit —amend”, then the edits show up as the revisions of the same change. (Which, while being a bit counterintuitive at first, makes the gerrit workflow more powerful than eg GitHub). —a I didn’t update classify.api and classify_api.c for the “Fancy_new_call” yet. In another Pull Request (PR), I’d like to develop a new plugin that uses the classify entry’s value and write packet tests etc. Cheers, Hemant From: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> <vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> > On Behalf Of Andrew Yourtchenko Sent: Tuesday, January 19, 2021 2:05 PM To: hem...@mnkcg.com <mailto:hem...@mnkcg.com> Cc: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> Subject: Re: [vpp-dev] classifier howto? Meta-comment: old_boring_call(foo) { /* meat */ } => Fancy_new_call(foo, bar) { /* new meat */ } Old_boring_call(foo) { Fancy_new_call(foo, 0); } This way you don’t have to patch umpteen unrelated places. --a On 19 Jan 2021, at 19:26, hemant via lists.fd.io <hemant=mnkcg....@lists.fd.io <mailto:hemant=mnkcg....@lists.fd.io> > wrote: Yay, I issued my first code review for VPP using gerrit for the issue of this email! https://gerrit.fd.io/r/c/vpp/+/30844 The JIRA issue I filed today is at: https://jira.fd.io/browse/VPP-1967 Thanks all for replying. Hemant From: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> <vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> > On Behalf Of hemant via lists.fd.io Sent: Monday, January 18, 2021 8:41 PM To: hem...@mnkcg.com <mailto:hem...@mnkcg.com> ; vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> Subject: Re: [vpp-dev] classifier howto? Please see this PR to fix what I need. https://github.com/FDio/vpp/pull/33 Please review – thanks. Hemant From: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> <vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> > On Behalf Of hemant via lists.fd.io Sent: Monday, January 18, 2021 10:52 AM To: vpp-dev@lists.fd.io <mailto:vpp-dev@lists.fd.io> Subject: [vpp-dev] classifier howto? I am used to the bihash’s easy to understand key-value pair API to program an entry for table lookup using the hash. https://github.com/FDio/vpp/blob/master/src/vppinfra/bihash_48_8.h#L39 The value in the struct is a u64. To see how I program a classifier entry for use by the data plane, I looked at the classifier entry data structure: https://github.com/FDio/vpp/blob/master/src/vnet/classify/vnet_classify.h#L66 The data structure uses a “u32x4 key[0];” key, but where is the value? I am used to seeing a key and value in an entry. Is “u32 opaque_index” the value? If yes, why does bihash use a “u64” for value but the classifier uses a u32? If the classifier table is hit, my next_node needs the “value” associated with the key that incurred the table hit. After all, for bihash, e.g., clib_bihash_search_48_8() API returns an explicit value. I plan to use the API in vnet_classify_add_del_session() to program ip4 and ip6 src and dst address prefix matching. Thanks, Hemant
smime.p7s
Description: S/MIME cryptographic signature
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#18567): https://lists.fd.io/g/vpp-dev/message/18567 Mute This Topic: https://lists.fd.io/mt/79925983/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
