From aa9f3f483c6b437e47f17de69dafd0d997a119cc Mon Sep 17 00:00:00 2001 From: IWASE Yusuke Date: Fri, 15 Dec 2017 10:00:09 +0900 Subject: [PATCH] ofproto_v1_5_parser: Fix length calculation of OFPOxmId Currently, the oxm_length field is always composed with zero value because OpenFlow Spec does not still clarify whether its value should be doubled if the hasmask bit is set. This patch fixes to compose the oxm_length value with the payload length because Open vSwitch strictly checks the oxm_length which contained in the OFPAT_COPY_FIELD action (introduced at OpenFlow 1.5), and this causes the OFPT_ERROR messages then flows will not be installed. Note: This patch does not backport this fix to ofproto_v1_3_parser.py and ofproto_v1_4_parser.py because those modules are tested based on the implementation of "linc/of_protocol" which supposes the oxm_length is always zero. Also OFPOxmId should be rarely serialized at the controller side when using OpenFlow 1.3 or 1.4. Signed-off-by: IWASE Yusuke Signed-off-by: FUJITA Tomonori --- ryu/ofproto/ofproto_v1_5_parser.py | 12 ++++++++---- ryu/ofproto/oxm_fields.py | 3 +++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/ryu/ofproto/ofproto_v1_5_parser.py b/ryu/ofproto/ofproto_v1_5_parser.py index c89bc37a..1146e5ce 100644 --- a/ryu/ofproto/ofproto_v1_5_parser.py +++ b/ryu/ofproto/ofproto_v1_5_parser.py @@ -2183,6 +2183,10 @@ class OFPTableFeaturePropNextTables(OFPTableFeatureProp): # oxm_length always 0 # ovs: # seems in flux as of writing this [20141003] +# updtate: OVS checks the oxm_length strictly which contained in +# the OFPAT_COPY_FIELD action when using OpenFlow 1.5, so here composes the +# payload length as the oxm_length (if has mask, it will be doubled, still +# unclear though). class OFPOxmId(StringifyMixin): _PACK_STR = '!I' # oxm header _EXPERIMENTER_ID_PACK_STR = '!I' @@ -2220,10 +2224,10 @@ class OFPOxmId(StringifyMixin): return cls(type_=type_, hasmask=hasmask, length=length), rest def serialize(self): - # fixup - self.length = 0 # XXX see the comment on OFPOxmId - - (n, _v, _m) = ofproto.oxm_from_user(self.type, None) + n, t = ofproto.oxm_get_field_info_by_name(self.type) + if not self.length: + # XXX see the comment on OFPOxmId + self.length = t.size * 2 if self.hasmask else t.size oxm = (n << (1 + 8)) | (self.hasmask << 8) | self.length buf = bytearray() msg_pack_into(self._PACK_STR, buf, 0, oxm) diff --git a/ryu/ofproto/oxm_fields.py b/ryu/ofproto/oxm_fields.py index 0c5e4597..d2bea495 100644 --- a/ryu/ofproto/oxm_fields.py +++ b/ryu/ofproto/oxm_fields.py @@ -65,6 +65,7 @@ # +-------------------------------+---------------+ from ryu.ofproto.oxx_fields import ( + _get_field_info_by_name, _from_user, _from_user_header, _to_user, @@ -177,6 +178,8 @@ def generate(modname): num_to_field = dict((f.num, f) for f in mod.oxm_types) # create functions by using oxx_fields module. + add_attr('oxm_get_field_info_by_name', + functools.partial(_get_field_info_by_name, oxx, name_to_field)) add_attr('oxm_from_user', functools.partial(_from_user, oxx, name_to_field)) add_attr('oxm_from_user_header',