On Mon, Jun 24, 2013 at 1:58 PM, Ben Pfaff <b...@nicira.com> wrote:
> experimenter IDs
> > > by adding new *_VENDOR_ID definitions to openflow-common.h.
> > >
> > > Signed-off-by: Ben Pfaff <b...@nicira.com>
>
> Please snip more when you add inline comments, so that I don't
> accidentally miss some of your questions among the quoted text.
> Thanks.
Sorry, will starting doing it.
> > > + if enum in reverse[version]:
> > > + error("%s: %s in OF%s means both %#x,%d,%d and "
> > > + "%#x,%d,%d." %
> > > + (dst, enum, version_reverse_map[version],
> > > + reverse[version][enum][0],
> > > + reverse[version][enum][1],
> > > + reverse[version][enum][2],
> > > + vendor, type_, code))
> > > + reverse[version][enum] = (vendor, type_, code)
> > >
> >
> >
> >
> > Want to ask here, I'm not sure whether "if enum in reverse[version]" will
> > ever be true. Because, if it is true, that means there are two exactly
> same
> > enum strings, which is impossible. The compiler will complain if two
> > entries in the enumeration have same name.
>
> I think you're right.
>
> Here's an incremental improvement, then:
>
Thanks, I'll check the new patch.
> > > OFPERR_OFPRRFC_BAD_ROLE,
> >
> >
> >
> > For NX, "(1,513)", do the type and code here correspond to "struct
> > nx_vendor_error 's type and code"?
>
> Yes.
>
> > What is the rule of defining NX code here?
>
> The "role" feature is a Nicira extension to OpenFlow that was adopted
> in OpenFlow 1.2 as a standard feature, so OpenFlow 1.2 and later have
> standard error codes for the feature.
>
> > Also, seems to me that now, the legit vendors (with assigned vendor id)
> can
> > freely define their own error messages, right?
>
> Yes, and in fact if you add another vendor ID to
> include/openflow/openflow-common.h then you can also add corresponding
> error codes in lib/ofp-errors.h.
>
Thanks, the explanation makes sense.
>
>
> > dnl Write-Metadata duplicated.
> > > -# bad OF1.1 instructions: OFPBAC_UNSUPPORTED_ORDER
> > > +# bad OF1.1 instructions: NXBIC_DUP_INSTRUCTION
> > > 0002 0018 00000000 fedcba9876543210 ff00ff00ff00ff00 0002 0018
> 00000000
> > > fedcba9876543210 ff00ff00ff00ff00
> >
> >
> > Want to ask what does NXBIC stand for? Also, I didn't find the definition
> > of this macro, even before applying patch 5/5. Want to know how and where
> > is it defined?
>
> Oops. This is a mistake. This change should not be in this patch.
> I've removed it.
I'm still curious why the test still passes. Could you explain more? Thanks
> > followed the code
> > and decoded it myself. And I really want to ask an unrelated question.
> >
> > I saw in the "ofperr_encode_msg__()" function in "lib/ofp-error.c", the
> > fields are converted to
> > network order, like below:
> >
> > "
> > oem = ofpbuf_put_uninit(buf, sizeof *oem);
> > oem->type = htons(NXET_VENDOR);
> > "
> >
> > In the "ovs_hex_dump()" function which prints the encoded header, it uses
> > "fprintf(stream, "%02hhx"
> > to print it and the value is again in host order (if %x is used, it will
> > print the network order).
> >
> > So I want to ask what is the use of "hhx" here? and why can't we get rid
> of
> > htons and use %x in
> > ovs_hex_dump?
>
> 'oem' is a "wire format" type, and its member 'type' is an ovs_be16,
> so to appropriately assign it a value it must be converted to network
> byte order first. That is why htons() is used.
>
> ovs_hex_dump() dumps the contents of a structure byte by byte. Its
> output will depend on the byte order of the underlying data. In this
> case the data is in network byte order, so the byte-by-byte dump will
> reflect that.
>
> 'hh' doesn't have anything to do with byte order. It means that the
> argument is a char type.
>
Thanks for the explanation. My previous experiment was wrong. And I found
out that the if "htons" is not used the "0xb0c2" when converted to "uint8
buf[2]" will be "buf[0]=0xc2" and "buf[1]=0xb0". So I think the "htons"
also helps preserve the hex order. right?
I'll repost this series since it has changed a bit.
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
