[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