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

[Jarno Rajahalme]

With two small nits below:

Acked-by: Jarno Rajahalme <jarno at ovn.org>

> 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?

Also, it looks like ‘—len’ takes two parameters ‘packet’ and ‘len’, use ‘packet_len’ instead.

 Jarno

>                              2, INT_MAX, netdev_dummy_receive, NULL);
>     unixctl_command_register("netdev-dummy/set-admin-state",
>                              "[netdev] up|down", 1, 2,
> -- 
> 1.9.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev