| Message ID | 79BBBFE6CB6C9B488C1A45ACD284F51961C17CB7@SHSMSX103.ccr.corp.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
I don't understand your explanation. decode_NXAST_RAW_DECAP() converts an OpenFlow action in wire format into OVS internal format. The OpenFlow action that it contains might have been generated by anything (for example, a controller). parse_ENCAP() doesn't have anything to do with it as far as I can tell. On Tue, Aug 01, 2017 at 10:45:50PM +0000, Yang, Yi Y wrote: > Ben, thank you for your patch, but from my understanding, parse_ENCAP has ensured it is impossible to have any property for decap, so I'm not sure in what case this will happened. > > -----Original Message----- > From: Ben Pfaff [mailto:blp@ovn.org] > Sent: Tuesday, August 1, 2017 11:26 PM > To: Yang, Yi Y <yi.y.yang@intel.com> > Cc: Zoltán Balogh <zoltan.balogh.eth@gmail.com>; dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v3 1/2] OF support and translation of generic encap and decap > > On Tue, Aug 01, 2017 at 12:32:20PM +0000, Yang, Yi Y wrote: > > #2. > > [Ben] I suspect that decode_NXAST_RAW_DECAP() should report an error if properties are present, since it doesn't support properties. > > > > [Yi] It is impossible. > > What is impossible? It is easy to detect that properties are present and report an error: > > diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index d0437f20922a..7be302a4005d 100644 > --- a/lib/ofp-actions.c > +++ b/lib/ofp-actions.c > @@ -4263,6 +4263,11 @@ decode_NXAST_RAW_DECAP(const struct nx_action_decap *nad, > enum ofp_version ofp_version OVS_UNUSED, > struct ofpbuf *ofpacts) { > + if (ntohs(nad->len) > sizeof *nad) { > + /* No properties supported yet. */ > + return OFPERR_OFPBPC_BAD_TYPE; > + } > + > struct ofpact_decap * decap; > > decap = ofpact_put_DECAP(ofpacts);
Got it, thanks a lot. -----Original Message----- From: Ben Pfaff [mailto:blp@ovn.org] Sent: Wednesday, August 2, 2017 6:51 AM To: Yang, Yi Y <yi.y.yang@intel.com> Cc: Zoltán Balogh <zoltan.balogh.eth@gmail.com>; dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v3 1/2] OF support and translation of generic encap and decap I don't understand your explanation. decode_NXAST_RAW_DECAP() converts an OpenFlow action in wire format into OVS internal format. The OpenFlow action that it contains might have been generated by anything (for example, a controller). parse_ENCAP() doesn't have anything to do with it as far as I can tell. On Tue, Aug 01, 2017 at 10:45:50PM +0000, Yang, Yi Y wrote: > Ben, thank you for your patch, but from my understanding, parse_ENCAP has ensured it is impossible to have any property for decap, so I'm not sure in what case this will happened. > > -----Original Message----- > From: Ben Pfaff [mailto:blp@ovn.org] > Sent: Tuesday, August 1, 2017 11:26 PM > To: Yang, Yi Y <yi.y.yang@intel.com> > Cc: Zoltán Balogh <zoltan.balogh.eth@gmail.com>; dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v3 1/2] OF support and translation of > generic encap and decap > > On Tue, Aug 01, 2017 at 12:32:20PM +0000, Yang, Yi Y wrote: > > #2. > > [Ben] I suspect that decode_NXAST_RAW_DECAP() should report an error if properties are present, since it doesn't support properties. > > > > [Yi] It is impossible. > > What is impossible? It is easy to detect that properties are present and report an error: > > diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index > d0437f20922a..7be302a4005d 100644 > --- a/lib/ofp-actions.c > +++ b/lib/ofp-actions.c > @@ -4263,6 +4263,11 @@ decode_NXAST_RAW_DECAP(const struct nx_action_decap *nad, > enum ofp_version ofp_version OVS_UNUSED, > struct ofpbuf *ofpacts) { > + if (ntohs(nad->len) > sizeof *nad) { > + /* No properties supported yet. */ > + return OFPERR_OFPBPC_BAD_TYPE; > + } > + > struct ofpact_decap * decap; > > decap = ofpact_put_DECAP(ofpacts);
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index d0437f20922a..7be302a4005d 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -4263,6 +4263,11 @@ decode_NXAST_RAW_DECAP(const struct nx_action_decap *nad, enum ofp_version ofp_version OVS_UNUSED, struct ofpbuf *ofpacts) { + if (ntohs(nad->len) > sizeof *nad) { + /* No properties supported yet. */ + return OFPERR_OFPBPC_BAD_TYPE; + } + struct ofpact_decap * decap; decap = ofpact_put_DECAP(ofpacts); _______________________________________________