With a question (or two) for clarification below:
Acked-by: Jarno Rajahalme <ja...@ovn.org>
> On Jan 18, 2016, at 11:26 PM, Ben Pfaff <b...@ovn.org> wrote:
>
> Until now it's been pretty hard to properly test any of the queue support,
> because the dummy network device doesn't have any queues. By adding one
> queue to the dummy network device (queue 0), we can get slightly higher
> confidence that OVS queue support works correctly. I suppose we could do
> even better if we made the dummy network device support modifying the
> queues, but that's a job for another day.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
> lib/netdev-dummy.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++----
> tests/ofproto.at | 83 +++++++++++++++++++++++----------------------
> 2 files changed, 136 insertions(+), 46 deletions(-)
>
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index df319de..c39f36c 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -1058,6 +1058,93 @@ netdev_dummy_get_stats(const struct netdev *netdev,
> struct netdev_stats *stats)
> }
>
> static int
> +netdev_dummy_get_queue(const struct netdev *netdev OVS_UNUSED,
> + unsigned int queue_id, struct smap *details)
> +{
> + if (queue_id == 0) {
> + smap_add(details, "key", "value”);
What is the function of adding this key/value-pair to the smap?
> + return 0;
> + } else {
> + return EINVAL;
> + }
> +}
> +
> +static void
> +netdev_dummy_init_queue_stats(struct netdev_queue_stats *stats)
> +{
> + *stats = (struct netdev_queue_stats) {
> + .tx_bytes = UINT64_MAX,
> + .tx_packets = UINT64_MAX,
> + .tx_errors = UINT64_MAX,
> + .created = LLONG_MIN,
> + };
> +}
> +
> +static int
> +netdev_dummy_get_queue_stats(const struct netdev *netdev OVS_UNUSED,
> + unsigned int queue_id,
> + struct netdev_queue_stats *stats)
> +{
> + if (queue_id == 0) {
> + netdev_dummy_init_queue_stats(stats);
> + return 0;
> + } else {
> + return EINVAL;
> + }
> +}
> +
> +struct netdev_dummy_queue_state {
> + unsigned int next_queue;
> +};
> +
> +static int
> +netdev_dummy_queue_dump_start(const struct netdev *netdev OVS_UNUSED,
> + void **statep)
> +{
> + struct netdev_dummy_queue_state *state = xmalloc(sizeof *state);
> + state->next_queue = 0;
> + *statep = state;
> + return 0;
> +}
> +
> +static int
> +netdev_dummy_queue_dump_next(const struct netdev *netdev OVS_UNUSED,
> + void *state_,
> + unsigned int *queue_id, struct smap *details)
> +{
> + struct netdev_dummy_queue_state *state = state_;
> + if (state->next_queue == 0) {
> + *queue_id = 0;
> + smap_add(details, "key", "value”);
Could this use ‘netdev_dummy_get_queue’ instead?
> + state->next_queue++;
> + return 0;
> + } else {
> + return EOF;
> + }
> +}
> +
> +static int
> +netdev_dummy_queue_dump_done(const struct netdev *netdev OVS_UNUSED,
> + void *state)
> +{
> + free(state);
> + return 0;
> +}
> +
> +static int
> +netdev_dummy_dump_queue_stats(const struct netdev *netdev OVS_UNUSED,
> + void (*cb)(unsigned int queue_id,
> + struct netdev_queue_stats *,
> + void *aux),
> + void *aux)
> +{
> + struct netdev_queue_stats stats;
> + netdev_dummy_init_queue_stats(&stats);
> + cb(0, &stats, aux);
> + return 0;
> +}
> +
> +static int
> netdev_dummy_get_ifindex(const struct netdev *netdev)
> {
> struct netdev_dummy *dev = netdev_dummy_cast(netdev);
> @@ -1147,14 +1234,14 @@ static const struct netdev_class dummy_class = {
> NULL, /* get_qos_capabilities */
> NULL, /* get_qos */
> NULL, /* set_qos */
> - NULL, /* get_queue */
> + netdev_dummy_get_queue,
> NULL, /* set_queue */
> NULL, /* delete_queue */
> - NULL, /* get_queue_stats */
> - NULL, /* queue_dump_start */
> - NULL, /* queue_dump_next */
> - NULL, /* queue_dump_done */
> - NULL, /* dump_queue_stats */
> + netdev_dummy_get_queue_stats,
> + netdev_dummy_queue_dump_start,
> + netdev_dummy_queue_dump_next,
> + netdev_dummy_queue_dump_done,
> + netdev_dummy_dump_queue_stats,
>
> netdev_dummy_get_in4, /* get_in4 */
> NULL, /* set_in4 */
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index f29543e..a4064d4 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -178,58 +178,53 @@ OFPST_PORT_DESC reply (OF1.5):
> OVS_VSWITCHD_STOP
> AT_CLEANUP
>
> -dnl This is really bare-bones.
> -dnl It at least checks request and reply serialization and deserialization.
> -AT_SETUP([ofproto - queue stats - (OpenFlow 1.0)])
> +dnl CHECK_QUEUE_STATS(label, option, format)
> +m4_define([CHECK_QUEUE_STATS], [
> +AT_SETUP([ofproto - queue stats - (OpenFlow $1)])
> OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl -vwarn queue-stats br0], [0], [stdout])
> -AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> -OFPST_QUEUE reply: 0 queues
> -])
> -AT_CHECK([ovs-ofctl -vwarn queue-stats br0 ANY 5], [0],
> - [OFPT_ERROR (xid=0x2): OFPQOFC_BAD_QUEUE
> -OFPST_QUEUE request (xid=0x2): port=ANY queue=5
> -])
> -AT_CHECK([ovs-ofctl -vwarn queue-stats br0 10], [0],
> - [OFPT_ERROR (xid=0x2): OFPQOFC_BAD_PORT
> -OFPST_QUEUE request (xid=0x2): port=10 queue=ALL
> +
> +AT_CHECK([ovs-ofctl -O $2 queue-stats br0 | STRIP_XIDS], [0],
> + [OFPST_QUEUE reply$3: 1 queues
> + port LOCAL queue 0: bytes=?, pkts=?, errors=?, duration=?
> ])
> -OVS_VSWITCHD_STOP
> -AT_CLEANUP
>
> -AT_SETUP([ofproto - queue stats - (OpenFlow 1.2)])
> -OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl -O OpenFlow12 -vwarn queue-stats br0], [0], [stdout])
> -AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> -OFPST_QUEUE reply (OF1.2): 0 queues
> +AT_CHECK([ovs-ofctl -O $2 queue-stats br0 LOCAL | STRIP_XIDS], [0],
> + [OFPST_QUEUE reply$3: 1 queues
> + port LOCAL queue 0: bytes=?, pkts=?, errors=?, duration=?
> ])
> -AT_CHECK([ovs-ofctl -O OpenFlow12 -vwarn queue-stats br0 ALL 5], [0],
> - [OFPT_ERROR (OF1.2) (xid=0x2): OFPQOFC_BAD_QUEUE
> -OFPST_QUEUE request (OF1.2) (xid=0x2): port=ANY queue=5
> +
> +AT_CHECK([ovs-ofctl -O $2 queue-stats br0 LOCAL 0 | STRIP_XIDS], [0],
> + [OFPST_QUEUE reply$3: 1 queues
> + port LOCAL queue 0: bytes=?, pkts=?, errors=?, duration=?
> ])
> -AT_CHECK([ovs-ofctl -O OpenFlow12 -vwarn queue-stats br0 10], [0],
> - [OFPT_ERROR (OF1.2) (xid=0x2): OFPQOFC_BAD_PORT
> -OFPST_QUEUE request (OF1.2) (xid=0x2): port=10 queue=ALL
> +
> +AT_CHECK([ovs-ofctl -O $2 queue-stats br0 ANY 0 | STRIP_XIDS], [0],
> + [OFPST_QUEUE reply$3: 1 queues
> + port LOCAL queue 0: bytes=?, pkts=?, errors=?, duration=?
> ])
> -OVS_VSWITCHD_STOP
> -AT_CLEANUP
>
> -AT_SETUP([ofproto - queue stats - (OpenFlow 1.4)])
> -OVS_VSWITCHD_START
> -AT_CHECK([ovs-ofctl -O OpenFlow14 -vwarn queue-stats br0], [0], [stdout])
> -AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> -OFPST_QUEUE reply (OF1.4): 0 queues
> +AT_CHECK([ovs-ofctl -O $2 queue-stats br0 LOCAL 5 | STRIP_XIDS], [0],
> + [OFPT_ERROR$3: OFPQOFC_BAD_QUEUE
> +OFPST_QUEUE request$3: port=LOCAL queue=5
> ])
> -AT_CHECK([ovs-ofctl -O OpenFlow14 -vwarn queue-stats br0 ALL 5], [0],
> - [OFPT_ERROR (OF1.4) (xid=0x2): OFPQOFC_BAD_QUEUE
> -OFPST_QUEUE request (OF1.4) (xid=0x2): port=ANY queue=5
> +
> +AT_CHECK([ovs-ofctl -O $2 queue-stats br0 ANY 5 | STRIP_XIDS], [0],
> + [OFPT_ERROR$3: OFPQOFC_BAD_QUEUE
> +OFPST_QUEUE request$3: port=ANY queue=5
> ])
> -AT_CHECK([ovs-ofctl -O OpenFlow14 -vwarn queue-stats br0 10], [0],
> - [OFPT_ERROR (OF1.4) (xid=0x2): OFPQOFC_BAD_PORT
> -OFPST_QUEUE request (OF1.4) (xid=0x2): port=10 queue=ALL
> +
> +AT_CHECK([ovs-ofctl -O $2 queue-stats br0 10 | STRIP_XIDS], [0],
> + [OFPT_ERROR$3: OFPQOFC_BAD_PORT
> +OFPST_QUEUE request$3: port=10 queue=ALL
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> +])
> +CHECK_QUEUE_STATS([1.0], [OpenFlow10], [])
> +CHECK_QUEUE_STATS([1.1], [OpenFlow11], [ (OF1.1)])
> +CHECK_QUEUE_STATS([1.2], [OpenFlow12], [ (OF1.2)])
> +CHECK_QUEUE_STATS([1.3], [OpenFlow13], [ (OF1.3)])
> +CHECK_QUEUE_STATS([1.4], [OpenFlow14], [ (OF1.4)])
>
> dnl This is really bare-bones.
> dnl It at least checks request and reply serialization and deserialization.
> @@ -239,11 +234,14 @@ ADD_OF_PORTS([br0], [1], [2])
> AT_CHECK([ovs-ofctl queue-get-config br0 1], [0], [stdout])
> AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> OFPT_QUEUE_GET_CONFIG_REPLY: port=1
> +queue 0:
> ])
> AT_CHECK([ovs-ofctl queue-get-config br0], [0], [stdout])
> AT_CHECK([STRIP_XIDS stdout | sort], [0], [dnl
> OFPT_QUEUE_GET_CONFIG_REPLY: port=1
> OFPT_QUEUE_GET_CONFIG_REPLY: port=2
> +queue 0:
> +queue 0:
> ])
> AT_CHECK([ovs-ofctl queue-get-config br0 10], [0],
> [OFPT_ERROR (xid=0x2): OFPQOFC_BAD_PORT
> @@ -258,6 +256,7 @@ ADD_OF_PORTS([br0], [1], [2])
> AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 1], [0], [stdout])
> AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> OFPT_QUEUE_GET_CONFIG_REPLY (OF1.2): port=1
> +queue 0:
> ])
> AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 10], [0],
> [OFPT_ERROR (OF1.2) (xid=0x2): OFPQOFC_BAD_PORT
> @@ -272,10 +271,14 @@ ADD_OF_PORTS([br0], [1], [2])
> AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 1], [0], [stdout])
> AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> OFPT_QUEUE_GET_CONFIG_REPLY (OF1.2): port=1
> +queue 0:
> ])
> AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 ANY], [0], [stdout])
> AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> OFPT_QUEUE_GET_CONFIG_REPLY (OF1.2): port=ANY
> +queue 0:
> +queue 0:
> +queue 0:
> ])
> AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 10], [0],
> [OFPT_ERROR (OF1.2) (xid=0x2): OFPQOFC_BAD_PORT
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
