Hi Damjan,

See my comments inline.

________________________________
From: Damjan Marion <dmar...@me.com>
Sent: Thursday, March 21, 2019 9:34 PM
To: Nitin Saxena
Cc: vpp-dev@lists.fd.io; Narayana Prasad Raju Athreya
Subject: Re: [EXT] Re: [vpp-dev] 128 byte cache line support



> On 21 Mar 2019, at 16:36, Nitin Saxena <nsax...@marvell.com> wrote:
>
> Hi Damjan,
>
> From: Damjan Marion <dmar...@me.com>
> Sent: Thursday, March 21, 2019 8:47 PM
> To: Nitin Saxena
> Cc: vpp-dev@lists.fd.io; Narayana Prasad Raju Athreya
> Subject: Re: [EXT] Re: [vpp-dev] 128 byte cache line support
>
>
>
>> On 21 Mar 2019, at 16:04, Nitin Saxena <nsax...@marvell.com> wrote:
>>
>> Hi Damjan,
>>
>> From: vpp-dev@lists.fd.io <vpp-dev@lists.fd.io> on behalf of Damjan Marion 
>> via Lists.Fd.Io <dmarion=me....@lists.fd.io>
>> Sent: Thursday, March 21, 2019 6:03 PM
>> To: Nitin Saxena
>> Cc: vpp-dev@lists.fd.io
>> Subject: Re: [EXT] Re: [vpp-dev] 128 byte cache line support
>>
>>
>>
>> > On 21 Mar 2019, at 06:51, Nitin Saxena <nsax...@marvell.com> wrote:
>> >
>> > Hi,
>> >
>> > First all sorry for responding late to this mail chain. Please see my 
>> > answers inline in blue
>> >
>> > Thanks,
>> > Nitin
>> >
>> >
>> > From: Damjan Marion <dmar...@me.com>
>> > Sent: Monday, March 18, 2019 4:48 PM
>> > To: Honnappa Nagarahalli
>> > Cc: vpp-dev; Nitin Saxena
>> > Subject: [EXT] Re: [vpp-dev] 128 byte cache line support
>> >
>> > External Email
>> >
>> >
>> >> On 15 Mar 2019, at 04:52, Honnappa Nagarahalli 
>> >> <honnappa.nagaraha...@arm.com> wrote:
>> >>
>> >>
>> >>
>> >> Related to change 18278[1], I was wondering if there is really a benefit 
>> >> of dealing with 128-byte cachelines like we do today.
>> >> Compiling VPP with cacheline size set to 128 will basically just add 64 
>> >> bytes of unused space at the end of each cacheline so
>> >> vlib_buffer_t for example will grow from 128 bytes to 256 bytes, but we 
>> >> will still need to prefetch 2 cachelines like we do by default.
>> >>
>> >> [Nitin]: This is the existing model. In case of forwarding mainly first 
>> >> vlib cache line size is being used. We are utilising existing hole (in 
>> >> first vlib cache line) by putting packet parsing info (Size ==64B). This 
>> >> has many benefits, one of them is to avoid ipv4-input-no-chksum() 
>> >> software checks. It gives us ~20 cycles benefits on our platform. So I do 
>> >> not want to lose that gain.
>>
>> That sounds like a terribly bad idea, and it likely will never be upstreamed.
>> vlib_buffer_t is 128-byte data structure, and it is perfect fit for 128-byte 
>> cacheline size systems. I don't see a point in growing this to 256-byte.
>> If you need more space, you can always grow headroom space for additional 
>> cacheline and store whatever you want there.
>> [Nitin]: In current upstream code, size of VLIB_BUFFER_HDR_SIZE is 256 byte 
>> for 128B cache line target and not 128 byte. So we haven't done any changes 
>> in the upstream code except introducing CLIB_MIN_CACHE_LINE_BYTES == 64 
>> macro and putting it across vlib_buffer_t as MARKER. So we are utilising 
>> existing hole (unused) in first cache line of vlib_buffer_t.
>
> I understood that, i’m proposing that we fix that so header size is always 
> 128, as it should be.
> [Nitin]: At this point I am not in favour of changing existing vlib_buffer_t 
> structure as I already mentioned that we gain ~20 cycles in case of L3Fwd. I 
> don't have data how much it would help me in TCP or UDP handling where we 
> would touch second cache line of vlib header.

What you are doing in your closed repo cannot be argument here.
We need to do what is right for everybody, not something to support your closed 
repo hacks.
[Nitin]: We have plan to upstream that piece of code. We are not willing to 
keep it closed as its inline with the upstream code. Currently we are in 
process of adding hardware mempool in VPP and native plugin for our target. We 
have plan to upstream those changes but we cannot upstream code while they are 
failing on our target.

If you plan to keep it closed source, you can do with that code whatever you 
want anyway.
If you want to upstream it, right thing to do is to discuss such changes before 
coding them.
[Nitin]: Actually I did discuss this piece of code, we have implemented 
"ipv4-input-nochksum-nottl-nodata" node which you and Dave had suggested.

>

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#12605): https://lists.fd.io/g/vpp-dev/message/12605
Mute This Topic: https://lists.fd.io/mt/30633927/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to