On Fri, Jun 30, 2017 at 03:29:29PM +0000, Zoltán Balogh wrote: > From: Jan Scheurich <jan.scheur...@ericsson.com> > > This commit adds support for the OpenFlow actions generic encap > and decap (as specified in ONF EXT-382) to the OVS control plane. > > CLI syntax for encap action with properties: > encap(hdr=<header>) > encap(hdr=<header>, > prop(class=<class>,type=<type>,val=<simple>), > prop(class=<class>,type=<type>,val(<complex>))) > > CLI syntax for decap action: > decap() > decap(packet_type(ns=<pt_ns>,type=<pt_type>)) > > The first header supported for encap and decap is "ethernet" to convert > packets between packet_type (1,Ethertype) and (0,0). > > Signed-off-by: Jan Scheurich <jan.scheur...@ericsson.com> > Signed-off-by: Yi Yang <yi.y.y...@intel.com>
Thanks for working on this. I have some comments. The patch was partially corrupted: lines in the patch that should have had a single space were changed to blank lines. This prevented the patch from applying without manual editing. I was able to work around it. (If you use "git send-email", this problem does not happen.) Please rename __OFPHTN_MAX to avoid the __ prefix, since that's reserved to the C implementation. Maybe OFPHTN_N_TYPES would be better. GCC says: ../lib/ofp-actions.c:4189:13: error: cast from 'char *' to 'struct ofpact_encap *' increases required alignment from 1 to 4 [-Werror,-Wcast-align] ../lib/ofp-actions.c:4222:16: error: cast from 'const uint8_t *' (aka 'const unsigned char *') to 'const struct ofpact_ed_prop *' increases required alignment from 1 to 2 [-Werror,-Wcast-align] ofp-actions.c is using OpenFlow action numbers 29 and 30, but there's no standardization of those values. I suggest using an NX extension number instead, since we control those directly in OVS. Same goes for the new OFPERR_OFPBAC_* errors. You can use NX extensions. In three places I see [0] used for a flexible array member. MSVC rejects this. You can use [] in place of [0]. In encode_ENCAP(), please put a space on each side of the binary operators here: + for (i=0; i<encap->n_props; i++) { Same in format_ed_props(). What is your plan for handling ofpact_check__() with decap actions? This patch is incomplete since it doesn't actually implement the encap or decap actions (in do_xlate_actions()). Maybe that's in a later patch. Thanks, Ben. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev