I checked the IETF tracker for NSH draft version 19:
It seems like final IETF reviews are concluding (deadline Aug 25), so we might 
consider updating the OVS implementation in 2.8 to align with version 19. At 
minimum this would mean restricting the nsh_mdtype field to 4 bits and possibly 
adjust the nsh_flags field to the changed bits in the base header.
I believe the we could add the nsh_ttl match field and a corresponding 
dec_nsh_ttl action at a later stage.
What do you think?
BR, Jan

From: Jan Scheurich
Sent: Wednesday, 16 August, 2017 16:05
To: Yang, Yi Y <yi.y.y...@intel.com>; Zoltán Balogh <zoltan.bal...@ericsson.com>
Cc: Ben Pfaff (b...@ovn.org) <b...@ovn.org>; Jiri Benc (jb...@redhat.com) 
<jb...@redhat.com>; ovs-dev@openvswitch.org
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

Hi Yi,
It is not correct that our implementation does not support MD2. OVS 2.8 will be 
able to receive and forward packets with NSH MD1 and MD2 headers (SFF use 
case). It should also be able to push an NSH header MD1 or MD2 format, with MD2 
TLVs specified as encap properties, (Classifier use case) and pop an NSH header 
with MD1 or MD2 format (last SFF use case).
The only limitation OVS 2.8 has is that it cannot match on or modify MD2 TLV 
metadata. This will require the introduction of generic TLV match fields to be 
mapped to specific TLV protocol headers, which we have postponed to a future 
release.
Regarding the Netlink representation of NSH key and PUSH_NSH action:
For the NSH key we agreed with Jiri and Ben that the nested OVS_KEY_ATTR_NSH 
must have separate members  OVS_NSH_KEY_ATTR_BASE and OVS_NSH_KEY_ATTR_MD1, as 
presence of MD1 is optional. A specific MD1 attribute is required to carry the 
four fixed context headers as key fields.
We do have a problem with the NSH_KEY_BASE as the base header remains a moving 
target in the NSH draft. The format has evolved as follows from

0                   1                   2                   3

0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1

+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

|Ver|O|C|R|R|R|R|R|R| Length    | MD type       | Next Protocol |

+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

in version 12 (Feb 2017), we based on, to
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|Ver|O|U|    TTL    |   Length  |U|U|U|U|MD Type| Next Protocol |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

in version 19 today (actually changed in June).
Field "nsh_mdtype" is shrunk from 8 to 4 bits, field "nsh_flags" would now 
split over two segments and shrunk from 8 to 2+4 bits, and there should be a 
new field "nsh_ttl".
So, whatever we release in OVS 2.8 will be compliant only to some version of 
the NSH draft (probably outdated). If we don't adjust to version 19, our 
"nsh_flags" field will contain the TTL value and the "nsh_mdtype" field 
includes some now unassigned bits. Will break as soon as future drafts start 
assigning bits previously part of the MD type field.
To me this indicates that we should mark the NSH implementation in OVS 2.8 as 
"experimental" and reserve the right to align the implementation with the 
approved NSH RFC later, even if that should break backward compatibility on the 
OpenFlow interface.
If that is not an option on the Netlink uAPI for the kernel datapath, we may 
not be able to offer NSH support with the kernel now. Or we accept that we need 
to supersede the initial OVS_NSH_KEY_ attributes with an updated version later 
on.

For the push_nsh action, the nested OVS_ACTION_ATTR_PUSH_NSH re-uses 
OVS_NSH_KEY_ATTR_BASE for the fixed part of the NSH header (inheriting the same 
issues as above).
But the push_nsh action needs no insight whatsoever in the context data. So the 
PUSH_NSH attribute just adds the opaque OVS_NSH_KEY_ATTR_CONTEXT carrying 
whatever context data as variable length byte array. This is a simple and 
future proof representation of arbitrarily formatted context data in the data 
path which can deal with both MD1 and MD2 already today.

BR Jan

From: Yang, Yi Y [mailto:yi.y.y...@intel.com]
Sent: Wednesday, 16 August, 2017 01:36
To: Jan Scheurich 
<jan.scheur...@ericsson.com<mailto:jan.scheur...@ericsson.com>>; Zoltán Balogh 
<zoltan.bal...@ericsson.com<mailto:zoltan.bal...@ericsson.com>>
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

Hi, Jan
I carefully considered netlink attributes, its core spirit is we shouldn't use 
variable-length netlink attribute if we can use fixed length, if you check 
OVS_KEY_ATTR_TUNNEL, that is a good example for this, actually that is just why 
Jiri, Eric and Ben hope we can change our implementation and follow this 
spirit, if we use OVS_NSH_KEY_ATTR to handle both MD1 and MD2, basically it 
violates this spirit, my another concern is we mustn't push more things about 
MD2 to this implementation, this will make people confused, we announced we 
can't support MD 2, why do we add some stuff here?
So my point is to use different netlink attributes for MD1 and MD2, that is 
clearer, to be important, MD1 attribute is length-fixed, for MD2, I don't think 
we have clear design for it, let us use another attribute to handle it 
specially. For implementation, what do you think the consolidated attribute 
OVS_NSH_KEY_ATTR_CONTEXT can bring?

From: Jan Scheurich [mailto:jan.scheur...@ericsson.com]
Sent: Wednesday, August 16, 2017 6:50 AM
To: Zoltán Balogh 
<zoltan.bal...@ericsson.com<mailto:zoltan.bal...@ericsson.com>>; Yang, Yi Y 
<yi.y.y...@intel.com<mailto:yi.y.y...@intel.com>>
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

I am suspecting the masked set action for NSH in the datapath flow

tunnel(tun_id=0x0,src=20.0.0.1,dst=20.0.0.2,flags(-df-csum+key)),recirc_id(0),in_port(4789),packet_type(ns=1,id=0x894f),nsh(spi=0x3020,si=255),
 packets:1, bytes:108, used:0.0s, 
actions:push_eth(src=00:00:00:00:00:00,dst=11:22:33:44:55:66),set(nsh(spi=0x3020,si=254)),pop_eth,
clone(tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=aa:55:00:00:00:03,src=aa:55:00:00:00:02,
dl_type=0x0800),ipv4(src=20.0.0.2,dst=20.0.0.3,proto=17,tos=0,ttl=64,frag=0x4000),
udp(src=0,dst=4789,csum=0x0),vxlan(flags=0xc000004,vni=0x0)),out_port(2)),
set(ipv4(src=30.0.0.2,dst=30.0.0.3)),tnl_pop(4789))
which rewrites the SI to 254 to cause the corruption of the NSH header. There 
is quite some complexity in
commit_nsh() assembling the nested ACTION_MASKED_SET_ATTR with the nested 
KEY_NSH_ATTR. I haven't checked the code executing the masked set action for 
NSH in the datapath either.
BR, Jan

From: Jan Scheurich
Sent: Wednesday, 16 August, 2017 00:17
To: Zoltán Balogh 
<zoltan.bal...@ericsson.com<mailto:zoltan.bal...@ericsson.com>>; 'Yang, Yi Y' 
<yi.y.y...@intel.com<mailto:yi.y.y...@intel.com>>
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

I fixed the crash and another bug that caused md2 printout to have excess 8 
bytes in push_nsh.
Changes pushed to gitlab.
Still the final triangle bridge test fails with a mismatch in datapath flows:
-tunnel(tun_id=0x0,src=30.0.0.2,dst=30.0.0.3,flags(-df-csum+key)),recirc_id(0),in_port(4789),packet_type(ns=1,id=0x894f),nsh(np=1,spi=0x3020,si=254),
 packets:1, bytes:108, used:0.0s, actions:pop_nsh(),recirc(0x2)
-tunnel(tun_id=0x0,src=30.0.0.2,dst=30.0.0.3,flags(-df-csum+key)),recirc_id(0x2),in_port(4789),packet_type(ns=1,id=0x800),ipv4(frag=no),
 packets:1, bytes:84, used:0.0s, 
actions:push_eth(src=00:00:00:00:00:00,dst=aa:55:aa:55:00:03),6
+tunnel(tun_id=0x0,src=30.0.0.2,dst=30.0.0.3,flags(-df-csum+key)),recirc_id(0),in_port(4789),packet_type(ns=1,id=0x894f),nsh(spi=0x300000,si=254),
 packets:1, bytes:108, used:0.0s, actions:drop
So the parsed NSH packet from the tunnel br-in2 to br-in3 is bad (missing np=1 
and wrong spi). Either the push_nsh action didn't work, or the parse_nsh 
function screwed up. The funny thing is that the nsh packet must have passed 
the first tunnel correctly.
BR, Jan

From: Zoltán Balogh
Sent: Tuesday, 15 August, 2017 20:57
To: Jan Scheurich 
<jan.scheur...@ericsson.com<mailto:jan.scheur...@ericsson.com>>; 'Yang, Yi Y' 
<yi.y.y...@intel.com<mailto:yi.y.y...@intel.com>>
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

Hi,

I can confirm. There are 3 nsh unit tests that do fail on Jan's last commit. I 
attached gdb and created a backtrace for each case:

## ------------------------------ ##
## openvswitch 2.8.90 test suite. ##
## ------------------------------ ##

2390: nsh - md2 encap over a veth link FAILED (nsh.at:211)
2389: nsh - md1 encap over a veth link FAILED (nsh.at:85)
2391: nsh - triangle PTAP bridge setup with NSH over vxlan-gpe FAILED 
(nsh.at:577)

## ------------- ##
## Test results. ##
## ------------- ##

ERROR: All 3 tests were run,
3 failed unexpectedly.
## -------------------------- ##
## testsuite.log was created. ##
## -------------------------- ##

Please send `tests/testsuite.log' and all information you think might help:

To: <b...@openvswitch.org<mailto:b...@openvswitch.org>>
Subject: [openvswitch 2.8.90] testsuite: 2389 2390 2391 failed

*** backtrace 2389

bt
#0 0x00007f18ca610428 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:54
#1 0x00007f18ca61202a in __GI_abort () at abort.c:89
#2 0x00000000004a84d5 in format_odp_push_nsh_action (push_nsh=0x7ffdf233ff40, 
ds=0x7ffdf2341830) at lib/odp-util.c:381
#3 format_odp_action (ds=ds@entry=0x7ffdf2341830, a=a@entry=0x1b1b588, 
portno_names=portno_names@entry=0x0) at lib/odp-util.c:1071
#4 0x00000000004a908e in format_odp_actions (ds=0x7ffdf2341830, 
actions=0x1b1b588, actions_len=100, portno_names=0x0) at lib/odp-util.c:1097
#5 0x000000000043a19a in ofproto_trace__ (ofproto=ofproto@entry=0x1af4790, 
flow=flow@entry=0x7ffdf2341850, packet=0x0, 
recirc_queue=recirc_queue@entry=0x7ffdf2341790, ofpacts=ofpacts@entry=0x0, 
ofpacts_len=ofpacts_len@entry=0, output=0x7ffdf2341830) at 
ofproto/ofproto-dpif-trace.c:588
#6 0x000000000043a335 in ofproto_trace (ofproto=0x1af4790, 
flow=flow@entry=0x7ffdf2341850, packet=<optimized out>, 
ofpacts=ofpacts@entry=0x0, ofpacts_len=ofpacts_len@entry=0, 
next_ct_states=next_ct_states@entry=0x7ffdf2341820, output=0x7ffdf2341830) at 
ofproto/ofproto-dpif-trace.c:632
#7 0x000000000043a906 in ofproto_unixctl_trace (conn=0x1b009e0, argc=<optimized 
out>, argv=<optimized out>, aux=<optimized out>) at 
ofproto/ofproto-dpif-trace.c:427
#8 0x0000000000515e21 in process_command (request=0x1afedd0, conn=0x1b009e0) at 
lib/unixctl.c:313
#9 run_connection (conn=0x1b009e0) at lib/unixctl.c:347
#10 unixctl_server_run (server=0x1abd440) at lib/unixctl.c:398
#11 0x0000000000406cdf in main (argc=10, argv=0x7ffdf2341cf8) at 
vswitchd/ovs-vswitchd.c:120

p push_nsh->md_type
$2 = 0 '\000'

*** backtrace 2390

bt
#0 0x00007f18859b6428 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:54
#1 0x00007f18859b802a in __GI_abort () at abort.c:89
#2 0x00000000004a84d5 in format_odp_push_nsh_action (push_nsh=0x7fff9307a1f0, 
ds=0x7fff9307bae0) at lib/odp-util.c:381
#3 format_odp_action (ds=ds@entry=0x7fff9307bae0, a=a@entry=0x1b4d468, 
portno_names=portno_names@entry=0x0) at lib/odp-util.c:1071
#4 0x00000000004a908e in format_odp_actions (ds=0x7fff9307bae0, 
actions=0x1b4d468, actions_len=104, portno_names=0x0) at lib/odp-util.c:1097
#5 0x000000000043a19a in ofproto_trace__ (ofproto=ofproto@entry=0x1b26790, 
flow=flow@entry=0x7fff9307bb00, packet=0x0, 
recirc_queue=recirc_queue@entry=0x7fff9307ba40, ofpacts=ofpacts@entry=0x0, 
ofpacts_len=ofpacts_len@entry=0, output=0x7fff9307bae0) at 
ofproto/ofproto-dpif-trace.c:588
#6 0x000000000043a335 in ofproto_trace (ofproto=0x1b26790, 
flow=flow@entry=0x7fff9307bb00, packet=<optimized out>, 
ofpacts=ofpacts@entry=0x0, ofpacts_len=ofpacts_len@entry=0, 
next_ct_states=next_ct_states@entry=0x7fff9307bad0, output=0x7fff9307bae0) at 
ofproto/ofproto-dpif-trace.c:632
#7 0x000000000043a906 in ofproto_unixctl_trace (conn=0x1b58a60, argc=<optimized 
out>, argv=<optimized out>, aux=<optimized out>) at 
ofproto/ofproto-dpif-trace.c:427
#8 0x0000000000515e21 in process_command (request=0x1b2c540, conn=0x1b58a60) at 
lib/unixctl.c:313
#9 run_connection (conn=0x1b58a60) at lib/unixctl.c:347
#10 unixctl_server_run (server=0x1aef440) at lib/unixctl.c:398
#11 0x0000000000406cdf in main (argc=10, argv=0x7fff9307bfa8) at 
vswitchd/ovs-vswitchd.c:120

p push_nsh->md_type
$2 = 0 '\000'

*** backtrace 2391

bt
#0 0x00007fba40955428 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:54
#1 0x00007fba4095702a in __GI_abort () at abort.c:89
#2 0x00000000004a84d5 in format_odp_push_nsh_action (push_nsh=0x7ffc1ef37420, 
ds=0x7ffc1ef37630) at lib/odp-util.c:381
#3 format_odp_action (ds=ds@entry=0x7ffc1ef37630, a=a@entry=0x18fee18, 
portno_names=portno_names@entry=0x0) at lib/odp-util.c:1071
#4 0x00000000004a908e in format_odp_actions (ds=0x7ffc1ef37630, 
actions=0x18fee14, actions_len=156, portno_names=0x0) at lib/odp-util.c:1097
#5 0x000000000056483f in format_dpif_flow (ds=<optimized out>, f=<optimized 
out>, ports=<optimized out>, type=<optimized out>, dpctl_p=<optimized out>) at 
lib/dpctl.c:762
#6 0x0000000000566b50 in dpctl_dump_flows (argc=<optimized out>, argc@entry=2, 
argv=argv@entry=0x18fb810, dpctl_p=dpctl_p@entry=0x7ffc1ef39c50) at 
lib/dpctl.c:923
#7 0x0000000000563f90 in dpctl_unixctl_handler (conn=0x18fb730, argc=2, 
argv=0x18fb810, aux=0x5667e0 <dpctl_dump_flows>) at lib/dpctl.c:2004
#8 0x0000000000515e21 in process_command (request=0x1900960, conn=0x18fb730) at 
lib/unixctl.c:313
#9 run_connection (conn=0x18fb730) at lib/unixctl.c:347
#10 unixctl_server_run (server=0x17e9440) at lib/unixctl.c:398
#11 0x0000000000406cdf in main (argc=10, argv=0x7ffc1ef39ea8) at 
vswitchd/ovs-vswitchd.c:120

p push_nsh->md_type
$2 = 144 '\220'

As you can see, the value of push_nsh->md_type is either 0x00 or 0x90 in 
format_odp_push_nsh_action() and this leads to abort.

Best regards,
Zoltan

-------- Original Message --------
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements
From: Jan Scheurich 
<jan.scheur...@ericsson.com<mailto:jan.scheur...@ericsson.com>>
Date: Aug 15, 2017, 18:03
To: "'Yang, Yi Y'" <yi.y.y...@intel.com<mailto:yi.y.y...@intel.com>>,Zoltán 
Balogh <zoltan.bal...@ericsson.com<mailto:zoltan.bal...@ericsson.com>>
Hi,
I have pushed my updates to a new branch "ovs_key_nsh_rework_jan" in gitlab.
The codes does compile but the NSH unit tests fail due to a crash of the 
vswitchd after ofproto/receive of an NSH packet.
I haven't had time to trouble-shoot and fix this yet.
Please look into that if you have time because I will be largely unavailable 
tomorrow.
BR, Jan

From: Jan Scheurich
Sent: Tuesday, 15 August, 2017 11:47
To: 'Yang, Yi Y' <yi.y.y...@intel.com<mailto:yi.y.y...@intel.com>>; Zoltán 
Balogh <zoltan.bal...@ericsson.com<mailto:zoltan.bal...@ericsson.com>>
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

Hi Yi,
I'll be providing my comments as a delta patch on your last commit.
As agreed I am changing the ATTR_ACTION_PUSH_NSH to a structured action 
attribute with ATTR_KEY_NSH_BASE and ATTR_KEY_NSH_CONTEXT, which is a flat 
variable length byte sequence. The push_nsh action does not need awareness of 
the MD type and structure. The MD length comes from the nested attribute length.
The kernel datapath doesn't compile with your latest commit as you have removed 
struct ovs_action_push_nsh from openvswitch.h which is used in the current 
versions of datapath/actions.c.
It seems also the datapath needs an optimized internal struct representation of 
the push_nsh action, and I think we should apply the same concept in the user 
space datapath. Just need to find out how.
BR, Jan

From: Jan Scheurich
Sent: Tuesday, 15 August, 2017 09:47
To: Yang, Yi Y <yi.y.y...@intel.com<mailto:yi.y.y...@intel.com>>; Zoltán Balogh 
<zoltan.bal...@ericsson.com<mailto:zoltan.bal...@ericsson.com>>
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

Hi Yi,
Thanks for providing the changes. I'm having a look right now. Since they are 
significant that might take me a while.
/Jan
From: Yang, Yi Y [mailto:yi.y.y...@intel.com]
Sent: Tuesday, 15 August, 2017 06:07
To: Zoltán Balogh 
<zoltan.bal...@ericsson.com<mailto:zoltan.bal...@ericsson.com>>; Jan Scheurich 
<jan.scheur...@ericsson.com<mailto:jan.scheur...@ericsson.com>>
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

I have fixed the issues, I'll send out kernel datapath patch v3 if you don't 
have other comments about new changes.

From: Zoltán Balogh [mailto:zoltan.bal...@ericsson.com]
Sent: Monday, August 14, 2017 11:52 PM
To: Jan Scheurich 
<jan.scheur...@ericsson.com<mailto:jan.scheur...@ericsson.com>>; Yang, Yi Y 
<yi.y.y...@intel.com<mailto:yi.y.y...@intel.com>>
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

Hello Yi,

I run make check on your branch and 3 tests failed:



## ------------- ##

## Test results. ##

## ------------- ##



ERROR: 2390 tests were run,

3 failed unexpectedly.

1 test was skipped.

## -------------------------- ##

## testsuite.log was created. ##

## -------------------------- ##



Please send `tests/testsuite.log' and all information you think might help:



   To: <b...@openvswitch.org<mailto:b...@openvswitch.org>>

   Subject: [openvswitch 2.8.90] testsuite: 953 997 2391 failed

Do these tests fail in your setup as well? Tomorrow, I'm going to continue with 
the debug.

Best regards,
Zoltan

From: Zoltán Balogh
Sent: Monday, August 14, 2017 4:13 PM
To: Jan Scheurich 
<jan.scheur...@ericsson.com<mailto:jan.scheur...@ericsson.com>>; Yang, Yi Y 
<yi.y.y...@intel.com<mailto:yi.y.y...@intel.com>>
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

Hi,
Sure, I can. I'll be back with updates.
/Zoltan

From: Jan Scheurich
Sent: Monday, August 14, 2017 4:08 PM
To: Yang, Yi Y <yi.y.y...@intel.com<mailto:yi.y.y...@intel.com>>
Cc: Zoltán Balogh 
<zoltan.bal...@ericsson.com<mailto:zoltan.bal...@ericsson.com>>
Subject: RE: Yang, Yi Y sent you a message in Skype for Business-FYI: basically 
I have worked out a ovs nsh version per their requirements

Hi Yi,
Zoltan is back from vacation today.
@Zoltan: Can you have a look at this issue and help Yi?
Thanks, Jan

From: Yang, Yi Y [mailto:yi.y.y...@intel.com]
Sent: Monday, 14 August, 2017 16:02
To: Yang, Yi Y <yi.y.y...@intel.com<mailto:yi.y.y...@intel.com>>; Jan Scheurich 
<jan.scheur...@ericsson.com<mailto:jan.scheur...@ericsson.com>>
Subject: Yang, Yi Y sent you a message in Skype for Business-FYI: basically I 
have worked out a ovs nsh version per their requirements

Yang, Yi Y 15:59:
Hi, Jan, I pushed origin/ovs_key_nsh_rework  to gitlab
Yang, Yi Y 16:00:
it will be great if you can have a check.
Yang, Yi Y 16:02:
nsh unit test 2391 failed, I'm still debugging it, I have no idea about why it 
is so, it will be great if you can help with this issue.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to