Hi Bill
Thank you for your report!
On 2017年04月05日 12:20, William Fisher wrote:
> When an OXM id is serialized in a table features request, the OXM
> header that is serialized sets the oxm_length to 0. That is, the OXM
> id for "eth_dst" is sent as
>
> 80000600 instead of 80000606 (the last byte is the length of the value).
>
> There's a comment in the code that explains the issue was unclear in
> 2014. (ofproto_v1_3_parser, ofproto_v1_4_parser, ofproto_v1_5_parser)
>
> The 1.3.5 OF spec states, "The oxm_length field in OXM headers must be
> the length value defined for the OXM field, i.e. the payload length if
> the OXM field had a payload." (7.3.5.5.2 Table Features properties)
>
> OVS appears to set the oxm_length to the payload length. If the mask
> bit is set, the payload length is double the usual size.
>
> I am trying to change the OFPOxmId serialization code so I can submit
> a patch. Is there an API like:
>
> ofproto.oxm_from_user('eth_dst', None)
>
> that will return the length information for a field?
Great!
It looks good to me.
ofproto.oxm_from_user() returns a tuple of ("oxm_type" value, serialized
value, serialized mask).
>>> ofproto.oxm_from_user('ipv4_src', ('192.168.0.0', '255.255.255.0'))
(4194315, b'\xc0\xa8\x00\x00', b'\xff\xff\xff\x00')
Instead, I guess we need to extend ofproto.oxm_serialize_header() to
include the "hasmask" field.
Just an idea, how about the following?
$ git diff
diff --git a/ryu/ofproto/oxx_fields.py b/ryu/ofproto/oxx_fields.py
index e9c1fb9..a3cc1d3 100644
--- a/ryu/ofproto/oxx_fields.py
+++ b/ryu/ofproto/oxx_fields.py
@@ -228,7 +228,7 @@ def _make_exp_hdr(oxx, mod, n):
return n, exp_hdr
-def _serialize_header(oxx, mod, n, buf, offset):
+def _serialize_header(oxx, mod, n, buf, offset, hasmask=False):
try:
get_desc = getattr(mod, '_' + oxx + '_field_desc')
desc = get_desc(n)
@@ -238,8 +238,10 @@ def _serialize_header(oxx, mod, n, buf, offset):
n, exp_hdr = _make_exp_hdr(oxx, mod, n)
exp_hdr_len = len(exp_hdr)
pack_str = "!I%ds" % (exp_hdr_len,)
+ if hasmask:
+ value_len *= 2
msg_pack_into(pack_str, buf, offset,
- (n << 9) | (0 << 8) | (exp_hdr_len + value_len),
+ (n << 9) | (hasmask << 8) | (exp_hdr_len + value_len),
bytes(exp_hdr))
return struct.calcsize(pack_str)
With this change, we can serialize oxm header field as following.
>>> from ryu.ofproto import ofproto_v1_3 as ofproto
>>> n = ofproto.oxm_from_user_header('ipv4_src')
>>> buf = bytearray()
>>> ofproto.oxm_serialize_header(n, buf, offset=0, hasmask=True)
4
>>> buf
bytearray(b'\x80\x00\x17\x08')
>>> ofproto.oxm_serialize_header(n, buf, offset=0, hasmask=False)
4
>>> buf
bytearray(b'\x80\x00\x16\x04')
Thanks,
Iwase
>
> The ryu parsing code for an OXM ID ignores the oxm_length. This patch
> would only break things if the receiving OpenFlow implementation
> expects the oxm_length's to be zero in the TableFeatures message
> OXMID's. Such an implementation would be broken IMO.
>
> -Bill
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Ryu-devel mailing list
> Ryu-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ryu-devel
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Ryu-devel mailing list
Ryu-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ryu-devel
