[ovs-dev] [PATCH v3 2/4] datapath: Unify vport error stats handling.

[Jesse Gross]

On Wed, May 1, 2013 at 4:02 PM, Pravin B Shelar <pshelar at nicira.com> wrote:
> Following patch changes vport->send return type so that vport
> layer can do error accounting.
>
> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>

This is definitely an improvement:
Acked-by: Jesse Gross <jesse at nicira.com>

Just one question:

> diff --git a/datapath/vport.h b/datapath/vport.h
> index cba578c..a393e54 100644
> --- a/datapath/vport.h
> +++ b/datapath/vport.h
> @@ -140,7 +140,8 @@ struct vport_parms {
>   * @get_name: Get the device's name.
>   * @get_config: Get the device's configuration.
>   * May be null if the device does not have an ifindex.
> - * @send: Send a packet on the device.  Returns the length of the packet sent.
> + * @send: Send a packet on the device.  Returns the length of the packet sent,
> + * zero for dropped packets or -ve for error.

What is -ve?

I think we probably also want to clarify the difference between an
error and dropped. To me dropped is when a packet is valid but some
kind of issue (such as out of memory) prevented transmission. So while
this looks like it faithfully maintains the current distinction
between errors and drops, I'm not sure that they were all right to
begin with.

I think we probably also could simplify some of the vport error code
(such as making it private to vport.c) and removing counters than
never get incremented now.