Re: [vpp-dev] N-queue node input

2019-03-22 Thread G. Paul Ziemba
Ah, thank you. That seems much more logical.

(Still familiarizing myself with tools in this toolbox. Haven't found the 10mm 
socket yet.)
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

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


Re: [EXT] [vpp-dev] 128 byte cache line support

2019-03-22 Thread Nitin Saxena


From: Damjan Marion 
Sent: Friday, March 22, 2019 3:59 PM
To: Nitin Saxena 
Cc: vpp-dev@lists.fd.io; Narayana Prasad Raju Athreya 
Subject: Re: [EXT] [vpp-dev] 128 byte cache line support




On 22 Mar 2019, at 09:35, Nitin Saxena 
mailto:nsax...@marvell.com>> wrote:

Hi Damjan,

>> Can we conclude that there is no technical arguments?

On technical aspects:

  1.  Changing VLIB structure to 128 byte from current 256 byte (for our target)

 *   In case of forwarding we only use single Dcache line till ipv4-input 
(because of packet parsing info in second 64B of vlib). The second cache line, 
which is data, being accessed only in ipv4-lookup and ipv4-rewrite node. With 
the proposal we have to put our hardware parsing information to HEADROOM, 
thereby increasing one cache line access to every node. This will impact 
performance for us.

I don't think this is technical argument, it falls to me under tecnical depth 
category.
Just for my curiosity, what’s wrong with putting your data into vlib_buffer_t-> 
opaque2 ?
[Nitin]: So 56 byte of opaque2 data should be fine if nobody overwrites to it 
in stack. In that case we prefer to have opaque2 data  at 64 byte aligned 
position (just after cacheline1 marker). Then I am fine with the proposal.


 *

  1.  Changing remaining per-thread structures to 128B

 *   This proposal looks fine for other data structures. However:
   i.  You 
already mentioned that in worst case there will be two prefetches of 64B but in 
our processor second prefetch should be ignored. I checked internally and yes 
the second prefetch will be ignored if the two prefetches are very close. 
However the second prefetch will not be ignored on the very first instruction 
pipeline. So I feel its wastage of few pipelining stages per packet. Otherwise 
it looks fine to me.

So here we just have your feeling, no real arguments, so i guess it is fine...

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

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


Re: [vpp-dev] Status of PPPoE Plugin

2019-03-22 Thread John Lo (loj) via Lists.Fd.Io
This part of the code mentioned let any MAC address with mcast bit through:
> +  /*if (!ethernet_address_cast (e0->dst_address) &&
>   (hi->hw_address != 0) &&
>   !eth_mac_equal ((u8 *) e0, hi->hw_address))
> error0 = ETHERNET_ERROR_L3_MAC_MISMATCH;

Regards,
John

From: Damjan Marion 
Sent: Friday, March 22, 2019 3:41 AM
To: Ni, Hongjun 
Cc: John Lo (loj) ; vpp-dev@lists.fd.io
Subject: Re: [vpp-dev] Status of PPPoE Plugin



On 22 Mar 2019, at 07:34, Ni, Hongjun 
mailto:hongjun...@intel.com>> wrote:
Hi John,

For first PADI packet, its destination MAC is broadcast address.
Not sure it is accepted by ethernet-input node?


Sure it does, otherwise arp, dhcp, ... will not work.


Do we need special process to handle it?
5.1 The PPPoE Active Discovery 
Initiation (PADI) packet


   The Host sends the PADI packet with the DESTINATION_ADDR set to the
   broadcast address.  The CODE field is set to 0x09 and the SESSION_ID
   MUST be set to 0x.

Thanks,
Hongjun

From: vpp-dev@lists.fd.io 
[mailto:vpp-dev@lists.fd.io] On Behalf Of John Lo (loj) via Lists.Fd.Io
Sent: Thursday, March 21, 2019 11:34 PM
To: dmar...@me.com; Ni, Hongjun 
mailto:hongjun...@intel.com>>
Cc: vpp-dev@lists.fd.io
Subject: Re: [vpp-dev] Status of PPPoE Plugin

Thanks for citing the PPPoE RFC 2516, Damjan.  The RFC goes on to describe how 
to resolve the MAC address for PPPoE sessions in Section 5 in discovery stage.  
As such, there is really no “L2 mode” for PPPoE sessions mentioned in my 
previous reply in this thread.  The root cause of the problem was MAC address 
resolution for PPPoE sessions.   -John

From: vpp-dev@lists.fd.io 
mailto:vpp-dev@lists.fd.io>> On Behalf Of Damjan Marion 
via Lists.Fd.Io
Sent: Thursday, March 21, 2019 11:08 AM
To: Ni, Hongjun mailto:hongjun...@intel.com>>
Cc: vpp-dev@lists.fd.io
Subject: Re: [vpp-dev] Status of PPPoE Plugin



> On 21 Mar 2019, at 06:20, Ni, Hongjun 
> mailto:hongjun...@intel.com>> wrote:
>
> Hi Raj,
>
> I think you may check the Ethernet type, if it is PPPoE packet, then MAC 
> check will be skipped.
>
> Thanks,
> Hongjun
>
> -Original Message-
> From: vpp-dev@lists.fd.io 
> [mailto:vpp-dev@lists.fd.io] On Behalf Of Raj
> Sent: Wednesday, March 20, 2019 9:48 PM
> To: vpp-dev mailto:vpp-dev@lists.fd.io>>
> Cc: alp.ars...@xflowresearch.com; Ni, 
> Hongjun mailto:hongjun...@intel.com>>
> Subject: Re: [vpp-dev] Status of PPPoE Plugin
>
> I commented out the offending parts and now PPPoE is working fine.
>
> diff --git a/src/vnet/ethernet/node.c b/src/vnet/ethernet/node.c index 
> 53d5b4eb0..7c5f673e0 100755
> --- a/src/vnet/ethernet/node.c
> +++ b/src/vnet/ethernet/node.c
> @@ -429,14 +429,14 @@ ethernet_input_inline (vlib_main_t * vm,
> }
>   else
> {
> +  /*if (!ethernet_address_cast (e0->dst_address) &&
>   (hi->hw_address != 0) &&
>   !eth_mac_equal ((u8 *) e0, hi->hw_address))
> error0 = ETHERNET_ERROR_L3_MAC_MISMATCH;
>   if (!ethernet_address_cast (e1->dst_address) &&
>   (hi->hw_address != 0) &&
>   !eth_mac_equal ((u8 *) e1, hi->hw_address))
> +error1 = ETHERNET_ERROR_L3_MAC_MISMATCH;*/
>   vlib_buffer_advance (b0, sizeof (ethernet_header_t));
>   determine_next_node (em, variant, 0, type0, b0,
>, ); @@ -653,10 +653,10 @@ 
> ethernet_input_inline (vlib_main_t * vm,
> }
>   else
> {
> +  /*if (!ethernet_address_cast (e0->dst_address) &&
>   (hi->hw_address != 0) &&
>   !eth_mac_equal ((u8 *) e0, hi->hw_address))
> +error0 = ETHERNET_ERROR_L3_MAC_MISMATCH;*/
>   vlib_buffer_advance (b0, sizeof (ethernet_header_t));
>   determine_next_node (em, variant, 0, type0, b0,
>, );
>
> I am sure this is not a patch which will be accepted upstream.

I would say: for sure not.

> I am not sure how to _correctly_ fix this, so that it will be accepted by 
> upstream. If some pointers are given I can submit a patch to get PPPoE 
> working correctly again in VPP.

This is typical case of “shotgun wedding” between control plane and dataplane 
over the TAP interface.

RFC2516 Section 4. clearly says:

   The DESTINATION_ADDR field contains either a unicast Ethernet
   destination address, or the Ethernet broadcast address (0x).
   For Discovery packets, the value is either a unicast or broadcast
   address as defined in the Discovery section.  For PPP session
   traffic, this field MUST contain the peer's unicast address as
   determined from the Discovery stage.

   The SOURCE_ADDR field MUST contains the Ethernet MAC address of the
   

Re: [vpp-dev] Status of PPPoE Plugin

2019-03-22 Thread Ni, Hongjun
Thanks Damjan. Will look into it and find some way to resolve it.

Thanks,
Hongjun

From: Damjan Marion [mailto:dmar...@me.com]
Sent: Friday, March 22, 2019 3:41 PM
To: Ni, Hongjun 
Cc: l...@cisco.com; vpp-dev@lists.fd.io
Subject: Re: [vpp-dev] Status of PPPoE Plugin



On 22 Mar 2019, at 07:34, Ni, Hongjun 
mailto:hongjun...@intel.com>> wrote:
Hi John,

For first PADI packet, its destination MAC is broadcast address.
Not sure it is accepted by ethernet-input node?


Sure it does, otherwise arp, dhcp, ... will not work.


Do we need special process to handle it?
5.1 The PPPoE Active Discovery 
Initiation (PADI) packet


   The Host sends the PADI packet with the DESTINATION_ADDR set to the
   broadcast address.  The CODE field is set to 0x09 and the SESSION_ID
   MUST be set to 0x.

Thanks,
Hongjun

From: vpp-dev@lists.fd.io 
[mailto:vpp-dev@lists.fd.io] On Behalf Of John Lo (loj) via Lists.Fd.Io
Sent: Thursday, March 21, 2019 11:34 PM
To: dmar...@me.com; Ni, Hongjun 
mailto:hongjun...@intel.com>>
Cc: vpp-dev@lists.fd.io
Subject: Re: [vpp-dev] Status of PPPoE Plugin

Thanks for citing the PPPoE RFC 2516, Damjan.  The RFC goes on to describe how 
to resolve the MAC address for PPPoE sessions in Section 5 in discovery stage.  
As such, there is really no “L2 mode” for PPPoE sessions mentioned in my 
previous reply in this thread.  The root cause of the problem was MAC address 
resolution for PPPoE sessions.   -John

From: vpp-dev@lists.fd.io 
mailto:vpp-dev@lists.fd.io>> On Behalf Of Damjan Marion 
via Lists.Fd.Io
Sent: Thursday, March 21, 2019 11:08 AM
To: Ni, Hongjun mailto:hongjun...@intel.com>>
Cc: vpp-dev@lists.fd.io
Subject: Re: [vpp-dev] Status of PPPoE Plugin



> On 21 Mar 2019, at 06:20, Ni, Hongjun 
> mailto:hongjun...@intel.com>> wrote:
>
> Hi Raj,
>
> I think you may check the Ethernet type, if it is PPPoE packet, then MAC 
> check will be skipped.
>
> Thanks,
> Hongjun
>
> -Original Message-
> From: vpp-dev@lists.fd.io 
> [mailto:vpp-dev@lists.fd.io] On Behalf Of Raj
> Sent: Wednesday, March 20, 2019 9:48 PM
> To: vpp-dev mailto:vpp-dev@lists.fd.io>>
> Cc: alp.ars...@xflowresearch.com; Ni, 
> Hongjun mailto:hongjun...@intel.com>>
> Subject: Re: [vpp-dev] Status of PPPoE Plugin
>
> I commented out the offending parts and now PPPoE is working fine.
>
> diff --git a/src/vnet/ethernet/node.c b/src/vnet/ethernet/node.c index 
> 53d5b4eb0..7c5f673e0 100755
> --- a/src/vnet/ethernet/node.c
> +++ b/src/vnet/ethernet/node.c
> @@ -429,14 +429,14 @@ ethernet_input_inline (vlib_main_t * vm,
> }
>   else
> {
> +  /*if (!ethernet_address_cast (e0->dst_address) &&
>   (hi->hw_address != 0) &&
>   !eth_mac_equal ((u8 *) e0, hi->hw_address))
> error0 = ETHERNET_ERROR_L3_MAC_MISMATCH;
>   if (!ethernet_address_cast (e1->dst_address) &&
>   (hi->hw_address != 0) &&
>   !eth_mac_equal ((u8 *) e1, hi->hw_address))
> +error1 = ETHERNET_ERROR_L3_MAC_MISMATCH;*/
>   vlib_buffer_advance (b0, sizeof (ethernet_header_t));
>   determine_next_node (em, variant, 0, type0, b0,
>, ); @@ -653,10 +653,10 @@ 
> ethernet_input_inline (vlib_main_t * vm,
> }
>   else
> {
> +  /*if (!ethernet_address_cast (e0->dst_address) &&
>   (hi->hw_address != 0) &&
>   !eth_mac_equal ((u8 *) e0, hi->hw_address))
> +error0 = ETHERNET_ERROR_L3_MAC_MISMATCH;*/
>   vlib_buffer_advance (b0, sizeof (ethernet_header_t));
>   determine_next_node (em, variant, 0, type0, b0,
>, );
>
> I am sure this is not a patch which will be accepted upstream.

I would say: for sure not.

> I am not sure how to _correctly_ fix this, so that it will be accepted by 
> upstream. If some pointers are given I can submit a patch to get PPPoE 
> working correctly again in VPP.

This is typical case of “shotgun wedding” between control plane and dataplane 
over the TAP interface.

RFC2516 Section 4. clearly says:

   The DESTINATION_ADDR field contains either a unicast Ethernet
   destination address, or the Ethernet broadcast address (0x).
   For Discovery packets, the value is either a unicast or broadcast
   address as defined in the Discovery section.  For PPP session
   traffic, this field MUST contain the peer's unicast address as
   determined from the Discovery stage.

   The SOURCE_ADDR field MUST contains the Ethernet MAC address of the
   source device.


So right thing to do here is to make sure that both control and session
packets are using single mac address, preferably MAC address of the VPP
interface where session traffic is received.



Re: [EXT] [vpp-dev] 128 byte cache line support

2019-03-22 Thread Damjan Marion via Lists.Fd.Io


> On 22 Mar 2019, at 09:35, Nitin Saxena  wrote:
> 
> Hi Damjan,
>  
> >> Can we conclude that there is no technical arguments?
>  
> On technical aspects:
> Changing VLIB structure to 128 byte from current 256 byte (for our target)
> In case of forwarding we only use single Dcache line till ipv4-input (because 
> of packet parsing info in second 64B of vlib). The second cache line, which 
> is data, being accessed only in ipv4-lookup and ipv4-rewrite node. With the 
> proposal we have to put our hardware parsing information to HEADROOM, thereby 
> increasing one cache line access to every node. This will impact performance 
> for us.

I don't think this is technical argument, it falls to me under tecnical depth 
category.
Just for my curiosity, what’s wrong with putting your data into vlib_buffer_t-> 
opaque2 ?


> Changing remaining per-thread structures to 128B
> This proposal looks fine for other data structures. However:
>i.  You 
> already mentioned that in worst case there will be two prefetches of 64B but 
> in our processor second prefetch should be ignored. I checked internally and 
> yes the second prefetch will be ignored if the two prefetches are very close. 
> However the second prefetch will not be ignored on the very first instruction 
> pipeline. So I feel its wastage of few pipelining stages per packet. 
> Otherwise it looks fine to me.

So here we just have your feeling, no real arguments, so i guess it is fine...

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

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


[vpp-dev] DHCPv6 proxy not working over VLAN interface

2019-03-22 Thread Raj
Hi all,

I am trying to assign v6 address to clients using DHCP PD. I have ISC
Bind configured to provide v6 prefixes and configured dhcpv6 proxy
using the following command.

vppctl set dhcpv6 proxy server 2001:xxx::10d::1 src-address
2001:xxx::10d::700

It is working for clients from non VLAN interface but fails for
clients in VLAN interface. It took some time to figure out but
ultimately the fix looked trivial.

diff --git a/src/vnet/dhcp/dhcp6_proxy_node.c b/src/vnet/dhcp/dhcp6_proxy_node.c
index 7d157ad35..40dc220d6 100644
--- a/src/vnet/dhcp/dhcp6_proxy_node.c
+++ b/src/vnet/dhcp/dhcp6_proxy_node.c
@@ -775,7 +775,7 @@ dhcpv6_proxy_to_client_input (vlib_main_t * vm,
 {
   u32 *vlan_tag = (u32 *) (mac0 + 1);
   u32 tmp;
-  tmp = (si0->sub.id << 16) | 0x0800;
+  tmp = (si0->sub.id << 16) | 0x86dd;
   *vlan_tag = clib_host_to_net_u32 (tmp);
 }

With this patch applied, DHCPv6 proxy is working correctly over VLANs.

I have couple of observations as I was testing the this feature.

1. Trace of proxy to client packets does not show any output interface
nodes and stops abruptly at dhcpv6-proxy-to-client node

14:49:51:731269: ip6-local
UDP: 2001:xxx::10d::1 -> 2001:xxx::10d::700
  tos 0x00, flow label 0x0, hop limit 64, payload length 219
UDP: 547 -> 547
  length 219, checksum 0xa3ef
14:49:51:731271: ip6-udp-lookup
  UDP: src-port 547 dst-port 547
14:49:51:731274: dhcpv6-proxy-to-client
  DHCPV6 proxy: sent to client from 2001:xxx::700::1
original_sw_if_index: 10, sw_if_index: 10

While the proxy to server packet trace shows next node as ip6-lookup
and it goes till TenGigabitEthernet85/0/1-tx

14:49:51:730918: dhcpv6-proxy-to-server
  DHCPV6 proxy: sent to server 2001:xxx::10d::1
original_sw_if_index: 10, sw_if_index: 10

14:49:51:730922: ip6-lookup
  fib 0 dpo-idx 4 flow hash: 0x
  UDP: 2001:xxx::10d::700 -> 2001:xxx::10d::1
tos 0x00, flow label 0x0, hop limit 32, payload length 163
  UDP: 546 -> 547

2. There is a comment in the code which shows a TODO item is pending
in the code:

/* $$$ consider adding a dynamic next to the graph node, for performance */

I could not figure out what the OP meant by this comment, but I can
attempt to do it if some one here can explain what needs to be done.

Is the abrupt ending of the trace and the above TODO related?

Thanks and Regards,

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

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


Re: [EXT] [vpp-dev] 128 byte cache line support

2019-03-22 Thread Nitin Saxena
Hi Damjan,

>> Can we conclude that there is no technical arguments?

On technical aspects:

  1.  Changing VLIB structure to 128 byte from current 256 byte (for our target)
 *   In case of forwarding we only use single Dcache line till ipv4-input 
(because of packet parsing info in second 64B of vlib). The second cache line, 
which is data, being accessed only in ipv4-lookup and ipv4-rewrite node. With 
the proposal we have to put our hardware parsing information to HEADROOM, 
thereby increasing one cache line access to every node. This will impact 
performance for us.
  2.  Changing remaining per-thread structures to 128B
 *   This proposal looks fine for other data structures. However:

   i.  You 
already mentioned that in worst case there will be two prefetches of 64B but in 
our processor second prefetch should be ignored. I checked internally and yes 
the second prefetch will be ignored if the two prefetches are very close. 
However the second prefetch will not be ignored on the very first instruction 
pipeline. So I feel its wastage of few pipelining stages per packet. Otherwise 
it looks fine to me.
Thanks,
Nitin


From: vpp-dev@lists.fd.io  On Behalf Of Damjan Marion via 
Lists.Fd.Io
Sent: Friday, March 22, 2019 1:32 PM
To: Nitin Saxena 
Cc: vpp-dev@lists.fd.io
Subject: Re: [EXT] [vpp-dev] 128 byte cache line support


—
Damjan

On 22 Mar 2019, at 01:37, Nitin Saxena 
mailto:nsax...@marvell.com>> wrote:




From: Damjan Marion mailto:dmar...@me.com>>
Sent: Friday, March 22, 2019 12:35 AM
To: Nitin Saxena
Cc: vpp-dev@lists.fd.io; Narayana Prasad Raju 
Athreya
Subject: Re: [EXT] [vpp-dev] 128 byte cache line support




On 21 Mar 2019, at 17:11, Nitin Saxena 
mailto:nsax...@marvell.com>> wrote:

Hi Damjan,

See my comments inline.


From: Damjan Marion mailto: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 
> mailto:nsax...@marvell.com>> wrote:
>
> Hi Damjan,
>
> From: Damjan Marion mailto: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 
>> mailto:nsax...@marvell.com>> wrote:
>>
>> Hi Damjan,
>>
>> From: vpp-dev@lists.fd.io 
>> mailto:vpp-dev@lists.fd.io>> on behalf of Damjan Marion 
>> via Lists.Fd.Io 
>> mailto: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 
>> > mailto: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 mailto: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 
>> >> mailto: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 

Re: [EXT] [vpp-dev] 128 byte cache line support

2019-03-22 Thread Damjan Marion via Lists.Fd.Io


— 
Damjan

> On 22 Mar 2019, at 01:37, Nitin Saxena  wrote:
> 
> 
> From: Damjan Marion 
> Sent: Friday, March 22, 2019 12:35 AM
> To: Nitin Saxena
> Cc: vpp-dev@lists.fd.io; Narayana Prasad Raju Athreya
> Subject: Re: [EXT] [vpp-dev] 128 byte cache line support
>  
> 
> 
>> On 21 Mar 2019, at 17:11, Nitin Saxena  wrote:
>> 
>> Hi Damjan,
>> 
>> See my comments inline.
>> 
>> From: Damjan Marion 
>> 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  wrote:
>> > 
>> > Hi Damjan,
>> > 
>> > From: Damjan Marion 
>> > 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  wrote:
>> >> 
>> >> Hi Damjan,
>> >> 
>> >> From: vpp-dev@lists.fd.io  on behalf of Damjan 
>> >> Marion via 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  wrote:
>> >> > 
>> >> > Hi,
>> >> > 
>> >> > First all sorry for responding late to this mail chain. Please see my 
>> >> > answers inline in blue
>> >> > 
>> >> > Thanks,
>> >> > Nitin
>> >> > 
>> >> > 
>> >> > From: Damjan Marion 
>> >> > 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 
>> >> >>  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. 
> 
> Storing data in vlib_buffer_t as you described above is definitely not 
> something we agreed on and is hardly something we will 

Re: [vpp-dev] Status of PPPoE Plugin

2019-03-22 Thread Damjan Marion via Lists.Fd.Io


> On 22 Mar 2019, at 07:34, Ni, Hongjun  wrote:
> 
> Hi John,
>  
> For first PADI packet, its destination MAC is broadcast address.
> Not sure it is accepted by ethernet-input node?


Sure it does, otherwise arp, dhcp, ... will not work.

> Do we need special process to handle it?
> 5.1 The PPPoE Active Discovery Initiation (PADI) packet
>  
>  
>The Host sends the PADI packet with the DESTINATION_ADDR set to the
>broadcast address.  The CODE field is set to 0x09 and the SESSION_ID
>MUST be set to 0x.
>  
> Thanks,
> Hongjun
>  
> From: vpp-dev@lists.fd.io [mailto:vpp-dev@lists.fd.io] On Behalf Of John Lo 
> (loj) via Lists.Fd.Io
> Sent: Thursday, March 21, 2019 11:34 PM
> To: dmar...@me.com; Ni, Hongjun 
> Cc: vpp-dev@lists.fd.io
> Subject: Re: [vpp-dev] Status of PPPoE Plugin
>  
> Thanks for citing the PPPoE RFC 2516, Damjan.  The RFC goes on to describe 
> how to resolve the MAC address for PPPoE sessions in Section 5 in discovery 
> stage.  As such, there is really no “L2 mode” for PPPoE sessions mentioned in 
> my previous reply in this thread.  The root cause of the problem was MAC 
> address resolution for PPPoE sessions.   -John
>  
> From: vpp-dev@lists.fd.io  On Behalf Of Damjan Marion 
> via Lists.Fd.Io
> Sent: Thursday, March 21, 2019 11:08 AM
> To: Ni, Hongjun 
> Cc: vpp-dev@lists.fd.io
> Subject: Re: [vpp-dev] Status of PPPoE Plugin
>  
> 
> 
> > On 21 Mar 2019, at 06:20, Ni, Hongjun  wrote:
> > 
> > Hi Raj,
> > 
> > I think you may check the Ethernet type, if it is PPPoE packet, then MAC 
> > check will be skipped.
> > 
> > Thanks,
> > Hongjun
> > 
> > -Original Message-
> > From: vpp-dev@lists.fd.io [mailto:vpp-dev@lists.fd.io] On Behalf Of Raj
> > Sent: Wednesday, March 20, 2019 9:48 PM
> > To: vpp-dev 
> > Cc: alp.ars...@xflowresearch.com; Ni, Hongjun 
> > Subject: Re: [vpp-dev] Status of PPPoE Plugin
> > 
> > I commented out the offending parts and now PPPoE is working fine.
> > 
> > diff --git a/src/vnet/ethernet/node.c b/src/vnet/ethernet/node.c index 
> > 53d5b4eb0..7c5f673e0 100755
> > --- a/src/vnet/ethernet/node.c
> > +++ b/src/vnet/ethernet/node.c
> > @@ -429,14 +429,14 @@ ethernet_input_inline (vlib_main_t * vm,
> > }
> >   else
> > {
> > +  /*if (!ethernet_address_cast (e0->dst_address) &&
> >   (hi->hw_address != 0) &&
> >   !eth_mac_equal ((u8 *) e0, hi->hw_address))
> > error0 = ETHERNET_ERROR_L3_MAC_MISMATCH;
> >   if (!ethernet_address_cast (e1->dst_address) &&
> >   (hi->hw_address != 0) &&
> >   !eth_mac_equal ((u8 *) e1, hi->hw_address))
> > +error1 = ETHERNET_ERROR_L3_MAC_MISMATCH;*/
> >   vlib_buffer_advance (b0, sizeof (ethernet_header_t));
> >   determine_next_node (em, variant, 0, type0, b0,
> >, ); @@ -653,10 +653,10 @@ 
> > ethernet_input_inline (vlib_main_t * vm,
> > }
> >   else
> > {
> > +  /*if (!ethernet_address_cast (e0->dst_address) &&
> >   (hi->hw_address != 0) &&
> >   !eth_mac_equal ((u8 *) e0, hi->hw_address))
> > +error0 = ETHERNET_ERROR_L3_MAC_MISMATCH;*/
> >   vlib_buffer_advance (b0, sizeof (ethernet_header_t));
> >   determine_next_node (em, variant, 0, type0, b0,
> >, );
> > 
> > I am sure this is not a patch which will be accepted upstream.
> 
> I would say: for sure not.
> 
> > I am not sure how to _correctly_ fix this, so that it will be accepted by 
> > upstream. If some pointers are given I can submit a patch to get PPPoE 
> > working correctly again in VPP.
> 
> This is typical case of “shotgun wedding” between control plane and dataplane 
> over the TAP interface.
> 
> RFC2516 Section 4. clearly says:
>
>The DESTINATION_ADDR field contains either a unicast Ethernet
>destination address, or the Ethernet broadcast address (0x).
>For Discovery packets, the value is either a unicast or broadcast
>address as defined in the Discovery section.  For PPP session
>traffic, this field MUST contain the peer's unicast address as
>determined from the Discovery stage.
> 
>The SOURCE_ADDR field MUST contains the Ethernet MAC address of the
>source device.
> 
> 
> So right thing to do here is to make sure that both control and session
> packets are using single mac address, preferably MAC address of the VPP
> interface where session traffic is received.
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> 
> View/Reply Online (#12600): https://lists.fd.io/g/vpp-dev/message/12600
> Mute This Topic: https://lists.fd.io/mt/28791570/675294
> Group Owner: vpp-dev+ow...@lists.fd.io
> Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub  [l...@cisco.com]
> -=-=-=-=-=-=-=-=-=-=-=-
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#12612): 

Re: [vpp-dev] Status of PPPoE Plugin

2019-03-22 Thread Ni, Hongjun
Hi John,

For first PADI packet, its destination MAC is broadcast address.
Not sure it is accepted by ethernet-input node?
Do we need special process to handle it?
5.1 The PPPoE Active Discovery 
Initiation (PADI) packet


   The Host sends the PADI packet with the DESTINATION_ADDR set to the
   broadcast address.  The CODE field is set to 0x09 and the SESSION_ID
   MUST be set to 0x.

Thanks,
Hongjun

From: vpp-dev@lists.fd.io [mailto:vpp-dev@lists.fd.io] On Behalf Of John Lo 
(loj) via Lists.Fd.Io
Sent: Thursday, March 21, 2019 11:34 PM
To: dmar...@me.com; Ni, Hongjun 
Cc: vpp-dev@lists.fd.io
Subject: Re: [vpp-dev] Status of PPPoE Plugin

Thanks for citing the PPPoE RFC 2516, Damjan.  The RFC goes on to describe how 
to resolve the MAC address for PPPoE sessions in Section 5 in discovery stage.  
As such, there is really no “L2 mode” for PPPoE sessions mentioned in my 
previous reply in this thread.  The root cause of the problem was MAC address 
resolution for PPPoE sessions.   -John

From: vpp-dev@lists.fd.io 
mailto:vpp-dev@lists.fd.io>> On Behalf Of Damjan Marion 
via Lists.Fd.Io
Sent: Thursday, March 21, 2019 11:08 AM
To: Ni, Hongjun mailto:hongjun...@intel.com>>
Cc: vpp-dev@lists.fd.io
Subject: Re: [vpp-dev] Status of PPPoE Plugin



> On 21 Mar 2019, at 06:20, Ni, Hongjun 
> mailto:hongjun...@intel.com>> wrote:
>
> Hi Raj,
>
> I think you may check the Ethernet type, if it is PPPoE packet, then MAC 
> check will be skipped.
>
> Thanks,
> Hongjun
>
> -Original Message-
> From: vpp-dev@lists.fd.io 
> [mailto:vpp-dev@lists.fd.io] On Behalf Of Raj
> Sent: Wednesday, March 20, 2019 9:48 PM
> To: vpp-dev mailto:vpp-dev@lists.fd.io>>
> Cc: alp.ars...@xflowresearch.com; Ni, 
> Hongjun mailto:hongjun...@intel.com>>
> Subject: Re: [vpp-dev] Status of PPPoE Plugin
>
> I commented out the offending parts and now PPPoE is working fine.
>
> diff --git a/src/vnet/ethernet/node.c b/src/vnet/ethernet/node.c index 
> 53d5b4eb0..7c5f673e0 100755
> --- a/src/vnet/ethernet/node.c
> +++ b/src/vnet/ethernet/node.c
> @@ -429,14 +429,14 @@ ethernet_input_inline (vlib_main_t * vm,
> }
>   else
> {
> +  /*if (!ethernet_address_cast (e0->dst_address) &&
>   (hi->hw_address != 0) &&
>   !eth_mac_equal ((u8 *) e0, hi->hw_address))
> error0 = ETHERNET_ERROR_L3_MAC_MISMATCH;
>   if (!ethernet_address_cast (e1->dst_address) &&
>   (hi->hw_address != 0) &&
>   !eth_mac_equal ((u8 *) e1, hi->hw_address))
> +error1 = ETHERNET_ERROR_L3_MAC_MISMATCH;*/
>   vlib_buffer_advance (b0, sizeof (ethernet_header_t));
>   determine_next_node (em, variant, 0, type0, b0,
>, ); @@ -653,10 +653,10 @@ 
> ethernet_input_inline (vlib_main_t * vm,
> }
>   else
> {
> +  /*if (!ethernet_address_cast (e0->dst_address) &&
>   (hi->hw_address != 0) &&
>   !eth_mac_equal ((u8 *) e0, hi->hw_address))
> +error0 = ETHERNET_ERROR_L3_MAC_MISMATCH;*/
>   vlib_buffer_advance (b0, sizeof (ethernet_header_t));
>   determine_next_node (em, variant, 0, type0, b0,
>, );
>
> I am sure this is not a patch which will be accepted upstream.

I would say: for sure not.

> I am not sure how to _correctly_ fix this, so that it will be accepted by 
> upstream. If some pointers are given I can submit a patch to get PPPoE 
> working correctly again in VPP.

This is typical case of “shotgun wedding” between control plane and dataplane 
over the TAP interface.

RFC2516 Section 4. clearly says:

   The DESTINATION_ADDR field contains either a unicast Ethernet
   destination address, or the Ethernet broadcast address (0x).
   For Discovery packets, the value is either a unicast or broadcast
   address as defined in the Discovery section.  For PPP session
   traffic, this field MUST contain the peer's unicast address as
   determined from the Discovery stage.

   The SOURCE_ADDR field MUST contains the Ethernet MAC address of the
   source device.


So right thing to do here is to make sure that both control and session
packets are using single mac address, preferably MAC address of the VPP
interface where session traffic is received.

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

View/Reply Online (#12600): https://lists.fd.io/g/vpp-dev/message/12600
Mute This Topic: https://lists.fd.io/mt/28791570/675294
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub  [l...@cisco.com]
-=-=-=-=-=-=-=-=-=-=-=-
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#12611):