[ovs-dev] [PATCH v4 1/6] Add a new OVS action check_pkt_larger

Numan Siddique nusiddiq at redhat.com
Mon Apr 22 19:25:01 UTC 2019


On Mon, Apr 22, 2019 at 11:52 PM Ben Pfaff <blp at ovn.org> wrote:

> On Mon, Apr 22, 2019 at 11:19:59AM -0700, Ben Pfaff wrote:
> > On Sat, Apr 20, 2019 at 01:07:13AM +0530, nusiddiq at redhat.com wrote:
> > > From: Numan Siddique <nusiddiq at redhat.com>
> > >
> > > This patch adds a new action 'check_pkt_larger' which checks if the
> > > packet is larger than the given size and stores the result in the
> > > destination register.
> > >
> > > Usage: check_pkt_larger(len)->REGISTER
> > > Eg. match=...,actions=check_pkt_larger(1442)->NXM_NX_REG0[0],next;
> > >
> > > This patch makes use of the new datapath action - 'check_pkt_len'
> > > which was recently added in the commit [1].
> > > At the start of ovs-vswitchd, datapath is probed for this action.
> > > If the datapath action is present, then 'check_pkt_larger'
> > > makes use of this datapath action.
> >
> > Thanks.  I found a few things to nitpick.  Here is my suggested
> > incremental to fold in for v5:
>
> Actually I forgot to save one change before I ran "git diff".  Here it
> is again:
>

Thanks for the review. Done.

Numan


>
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index ec69a4377424..1a24063d087c 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -7511,7 +7511,7 @@ parse_CHECK_PKT_LARGER(char *arg, const struct
> ofpact_parse_params *pp)
>      }
>
>      delim[0] = '\0';
> -    if (value[strlen(value) - 1] ==')') {
> +    if (value[strlen(value) - 1] == ')') {
>          value[strlen(value) - 1] = '\0';
>      }
>      struct mf_subfield dst;
> diff --git a/lib/ovs-actions.xml b/lib/ovs-actions.xml
> index 935845ba30c7..cfd9b81be604 100644
> --- a/lib/ovs-actions.xml
> +++ b/lib/ovs-actions.xml
> @@ -1462,14 +1462,14 @@ $ ovs-ofctl -O OpenFlow10 add-flow br0
> actions=mod_nw_src:1.2.3.4
>
>        <p>
>          Checks if the packet is larger than the specified length in
> -        <var>pkt_len</var> and stores the result <code>1</code> in the
> -        OpenFlow register field 1-bit specified in <var>dst</var>.
> +        <var>pkt_len</var>.  If so, stores 1 in <var>dst</var>, which
> should be
> +        a 1-bit field; if not, stores 0.
>        </p>
>
>        <p>
>          The packet length to check againt the argument <var>pkt_len</var>
> -        includes the L2 header and L2 payload of the packet. But
> -        it doesn't include the vlan tag if present.
> +        includes the L2 header and L2 payload of the packet, but not the
> VLAN
> +        tag (if present).
>        </p>
>
>        <p>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 7722bbed5950..e6af0fc01dab 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1327,8 +1327,8 @@ check_check_pkt_len(struct dpif_backer *backer)
>                                     &actions, NULL);
>      ofpbuf_uninit(&actions);
>      VLOG_INFO("%s: Datapath %s check_pkt_len action",
> -              dpif_name(backer->dpif), (supported) ? "supports"
> -                                                   : "does not support");
> +              dpif_name(backer->dpif), supported ? "supports"
> +                                                 : "does not support");
>      return supported;
>  }
>
> diff --git a/tests/odp.at b/tests/odp.at
> index 3173e30a93ba..8e4ba4615548 100644
> --- a/tests/odp.at
> +++ b/tests/odp.at
> @@ -107,7 +107,6 @@ sed -i'back'
> 's/\(in_port(1)\),\(eth\)/\1,packet_type(ns=0,id=0),\2/' odp-out.tx
>
>  AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [`cat
> odp-out.txt`
>  ])
> -
>  AT_CLEANUP
>
>  AT_SETUP([OVS datapath wildcarded key parsing and formatting - valid
> forms])
>


More information about the dev mailing list