Looks good.
Ethan
On Thu, Sep 8, 2011 at 12:36, Ben Pfaff <b...@nicira.com> wrote:
> Some invalid ports (those above the maximum port number supported by the
> datapath, including OpenFlow reserved ports that are not translated by OVS
> into some other number) will be rejected by the datapath. It's better to
> catch these early and send back an appropriate OpenFlow error code, rather
> than to just get EINVAL from the kernel and have to guess at the problem.
>
> Reported-by: Aaron Rosen <aro...@clemson.edu>
> ---
> include/openflow/nicira-ext.h | 3 +++
> ofproto/ofproto-dpif.c | 4 ++++
> ofproto/ofproto.c | 13 ++++++++++++-
> 3 files changed, 19 insertions(+), 1 deletions(-)
>
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> index 890e974..0d68fb3 100644
> --- a/include/openflow/nicira-ext.h
> +++ b/include/openflow/nicira-ext.h
> @@ -110,6 +110,9 @@ enum nx_bad_request_code {
>
> /* NXT_ROLE_REQUEST specified an invalid role. */
> NXBRC_BAD_ROLE = 0x201,
> +
> + /* The in_port in an ofp_packet_out request is invalid. */
> + NXBRC_BAD_IN_PORT = 0x202
> };
>
> /* Additional "code" values for OFPET_FLOW_MOD_FAILED. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index f09c230..54dd7a2 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3920,6 +3920,10 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf
> *packet,
> struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> int error;
>
> + if (flow->in_port >= ofproto->max_ports && flow->in_port < OFPP_MAX) {
> + return ofp_mkerr_nicira(OFPET_BAD_REQUEST, NXBRC_BAD_IN_PORT);
> + }
> +
> error = validate_actions(ofp_actions, n_ofp_actions, flow,
> ofproto->max_ports);
> if (!error) {
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 156dc90..355e46d 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -1575,6 +1575,7 @@ handle_packet_out(struct ofconn *ofconn, const struct
> ofp_header *oh)
> struct ofpbuf request;
> struct flow flow;
> size_t n_ofp_actions;
> + uint16_t in_port;
> int error;
>
> COVERAGE_INC(ofproto_packet_out);
> @@ -1608,8 +1609,18 @@ handle_packet_out(struct ofconn *ofconn, const struct
> ofp_header *oh)
> buffer = NULL;
> }
>
> + /* Get in_port and partially validate it.
> + *
> + * We don't know what range of ports the ofproto actually implements, but
> + * we do know that only certain reserved ports (numbered OFPP_MAX and
> + * above) are valid. */
> + in_port = ntohs(opo->in_port);
> + if (in_port >= OFPP_MAX && in_port != OFPP_LOCAL && in_port !=
> OFPP_NONE) {
> + return ofp_mkerr_nicira(OFPET_BAD_REQUEST, NXBRC_BAD_IN_PORT);
> + }
> +
> /* Send out packet. */
> - flow_extract(&payload, 0, ntohs(opo->in_port), &flow);
> + flow_extract(&payload, 0, in_port, &flow);
> error = p->ofproto_class->packet_out(p, &payload, &flow,
> ofp_actions, n_ofp_actions);
> ofpbuf_delete(buffer);
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
