[ovs-dev] [userspace meter v3 1/5] netdev-dummy: Add --len option for netdev-dummy/receive command

Andy Zhou azhou at ovn.org
Thu Jan 26 23:01:21 UTC 2017


On Wed, Jan 25, 2017 at 4:16 PM, Jarno Rajahalme <jarno at ovn.org> wrote:

> With two small nits below:
>
> Acked-by: Jarno Rajahalme <jarno at ovn.org>
>

Thanks.

>
> > On Jan 24, 2017, at 10:45 PM, Andy Zhou <azhou at ovn.org> wrote:
> >
> > Currently, there is no way to specify the packet size when injecting
> > a packet via "netdev-dummy/receive" with a flow specification. Thus
> > far, packet size is not important for testing OVS features, but it
> > becomes useful in writing unit tests for the meter implementation
> > in a later patch.
> >
> > Signed-off-by: Andy Zhou <azhou at ovn.org>
> > ---
> > lib/netdev-dummy.c | 38 ++++++++++++++++++++++++++++----------
> > 1 file changed, 28 insertions(+), 10 deletions(-)
> >
> > diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> > index e6e36cd..10df0a7 100644
> > --- a/lib/netdev-dummy.c
> > +++ b/lib/netdev-dummy.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2010, 2011, 2012, 2013, 2015, 2016 Nicira, Inc.
> > + * Copyright (c) 2010, 2011, 2012, 2013, 2015, 2016, 2017 Nicira, Inc.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -1433,7 +1433,15 @@ pkt_list_delete(struct ovs_list *l)
> > }
> >
> > static struct dp_packet *
> > -eth_from_packet_or_flow(const char *s)
> > +eth_from_packet(const char *s)
> > +{
> > +    struct dp_packet *packet;
> > +    eth_from_hex(s, &packet);
> > +    return packet;
> > +}
> > +
> > +static struct dp_packet *
> > +eth_from_flow(const char *s)
> > {
> >     enum odp_key_fitness fitness;
> >     struct dp_packet *packet;
> > @@ -1441,10 +1449,6 @@ eth_from_packet_or_flow(const char *s)
> >     struct flow flow;
> >     int error;
> >
> > -    if (!eth_from_hex(s, &packet)) {
> > -        return packet;
> > -    }
> > -
> >     /* Convert string to datapath key.
> >      *
> >      * It would actually be nicer to parse an OpenFlow-like flow key
> here, but
> > @@ -1540,10 +1544,24 @@ netdev_dummy_receive(struct unixctl_conn *conn,
> >     for (i = k; i < argc; i++) {
> >         struct dp_packet *packet;
> >
> > -        packet = eth_from_packet_or_flow(argv[i]);
> > +        /* Try to parse 'argv[i]' as packet in hex. */
> > +        packet = eth_from_packet(argv[i]);
> > +
> >         if (!packet) {
> > -            unixctl_command_reply_error(conn, "bad packet syntax");
> > -            goto exit;
> > +            /* Try parse 'argv[i]' as odp flow. */
> > +            packet = eth_from_flow(argv[i]);
> > +
> > +            if (!packet) {
> > +                unixctl_command_reply_error(conn, "bad packet or flow
> syntax");
> > +                goto exit;
> > +            }
> > +
> > +            /* Parse optional --len argument immediately follow a
> 'flow'.  */
> > +            if (argc >= i + 2 && !strcmp(argv[i + 1], "--len")) {
> > +                int packet_size = strtol(argv[i + 2], NULL, 10);
> > +                dp_packet_set_size(packet, packet_size);
> > +                i+=2;
> > +            }
> >         }
> >
> >         netdev_dummy_queue_packet(dummy_dev, packet, rx_qid);
> > @@ -1757,7 +1775,7 @@ void
> > netdev_dummy_register(enum dummy_level level)
> > {
> >     unixctl_command_register("netdev-dummy/receive",
> > -                             "name [--qid queue_id] packet|flow...",
> > +                             "name [--qid queue_id] packet|flow [--len
> packet len]..”,
>
> Not sure of the ‘..’ after the [—len], as we do not accept anything after
> ‘—len’s parameter?
>
I think it refers to the fact we can accepts multiple packets at a time. I
dropped it to avoid confusion. Most calling sites
sends one packet at time any ways.

>
> Also, it looks like ‘—len’ takes two parameters ‘packet’ and ‘len’, use
> ‘packet_len’ instead.
>
> Fixed.   Pushed to master.


More information about the dev mailing list