On Wed, Oct 16, 2013 at 09:09:15AM -0700, Ben Pfaff wrote:
> On Wed, Oct 16, 2013 at 07:17:01PM +0900, Simon Horman wrote:
> > This alters the way rx packets are accounted for by
> > counting them when they are processed by netdev_dummy_rx_recv(),
> > which seems to be a common path used by all received packets.
> > 
> > Previously accounting was done earlier, in netdev_dummy_receive(),
> > however this does not appear to count packets that are received via
> > a socket.
> > 
> > This resolves packet counting errors reported by the following
> > OFtest tests:
> > 
> >     port_stats.MultiFlowStats
> >     port_stats.SingleFlowStats
> >     pktact.WildcardPriorityWithDelete
> >     pktact.WildcardPriority
> > 
> > Signed-off-by: Simon Horman <ho...@verge.net.au>
> 
> Hey, thanks a lot!
> 
> This yielded compile-time errors:
> 
>     ../lib/netdev-dummy.c:471:60: error: incompatible pointer types passing 
> 'struct netdev_dummy *' to parameter of type 'const struct netdev *' 
> [-Werror,-Wincompatible-pointer-types]
>             struct netdev_dummy *dummy_dev = netdev_dummy_cast(netdev);
>                                                                ^~~~~~
>     ../lib/netdev-dummy.c:107:40: note: passing argument to parameter 
> 'netdev' here
>     netdev_dummy_cast(const struct netdev *netdev)

Sorry, that was careless.

> I fixed them:
> 
> diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> index 061ad0c..fe54576 100644
> --- a/lib/netdev-dummy.c
> +++ b/lib/netdev-dummy.c
> @@ -468,15 +468,13 @@ netdev_dummy_rx_recv(struct netdev_rx *rx_, void 
> *buffer, size_t size)
>      }
>  
>      if (packet->size <= size) {
> -        struct netdev_dummy *dummy_dev = netdev_dummy_cast(netdev);
> -
>          memcpy(buffer, packet->data, packet->size);
>          retval = packet->size;
> -        ovs_mutex_lock(&dummy_dev->mutex);
> -        dummy_dev->stats.rx_packets++;
> -        dummy_dev->stats.rx_bytes += packet->size;
> -        ovs_mutex_unlock(&dummy_dev->mutex);
>  
> +        ovs_mutex_lock(&netdev->mutex);
> +        netdev->stats.rx_packets++;
> +        netdev->stats.rx_bytes += packet->size;
> +        ovs_mutex_unlock(&netdev->mutex);
>      } else {
>          retval = -EMSGSIZE;
>      }
> 
> and I applied it to master with that change.

I did wonder if this solution is slightly sub-optimal as
now the mutex is taken both in netdev_dummy_rx_recv()
and netdev_dummy_rx_recv(). Then again, I'm not sure that
it is critical to optimise netdev-dummy.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to