> > >71034a0..faf3583 100644
> > >--- a/lib/netdev-dpdk.c
> > >+++ b/lib/netdev-dpdk.c
> > >@@ -53,6 +53,7 @@
> > >
> > > #include "rte_config.h"
> > > #include "rte_mbuf.h"
> > >+#include "rte_meter.h"
> > > #include "rte_virtio_net.h"
> > >
> > > VLOG_DEFINE_THIS_MODULE(dpdk);
> > >@@ -193,6 +194,11 @@ struct dpdk_ring {
> > >     struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);  };
> > >
> > >+struct ingress_policer {
> > >+    struct rte_meter_srtcm_params app_srtcm_params;
> > >+    struct rte_meter_srtcm in_policer; };
> > >+
> > > struct netdev_dpdk {
> > >     struct netdev up;
> > >     int port_id;
> > >@@ -231,6 +237,13 @@ struct netdev_dpdk {
> > >     /* Identifier used to distinguish vhost devices from each other
> */
> > >     char vhost_id[PATH_MAX];
> > >
> > >+    /* Ingress Policer */
> > >+    rte_spinlock_t policer_lock;
> > >+    struct ingress_policer *ingress_policer;
> > >+
> >
> > I would prefer not to have a lock at this level.
> >
> > I think it would make more sense to make the ingress_policer pointer
> > RCU protected and embed the spinlock into struct ingress_policer, to
> > protect the token bucket.
> >
> Sure I agree, I was modelling this on what we have currently in QoS just
> for a rough implementation.
> I can take a look at using RCU instead. This could possibly be extended
> to the QoS use case also in the futue.

Hi Daniele, I have been looking at an RCU implementation here but so far have 
not been able to get it working correctly.

The issue I'm seeing is when I destroy the policer while traffic is passing a 
segfault sometimes occurs as the meter is in use when the ingress policer is 
set to NULL.

I'm pretty sure this is down to my understanding (or lack thereof) of the 
ovsrcu behavior.

Is the following high level implementation correct in your eyes?

The ingress policer struct is as follows:

struct ingress_policer {
    struct rte_meter_srtcm_params app_srtcm_params;
    struct rte_meter_srtcm in_policer;
};

From your comment above you mention embedding the spinlock in the ingress 
policer struct.
Just to clarify, does the rcu by nature embed a spinlock or did you mean move 
the rte_spinlock policer_lock from the netdev_dpdk struct into the ingress 
policer struct?

Is the behavior you are thinking of something like the following for when 
traffic is being processed?

1. Get the rcu ingress_policer pointer.
2. Lock the spinlock in the ingress policer struct.
3. Set the ovsrcu pointer
4. Call ovsrcu_synchronize to that all threads see that the policer is locked 
(Stop threads from accessing the ingress policer)
5. Process the packets in the meter as usual.
6. Unlock the spinlock.
7. Set the ovsrcu pointer
6. Synchronize again? (So that threads can access the ingress policer again)

For destroying the ingress policer

1. Get the rcu ingress_policer pointer.
2. Lock the spinlock in the ingress policer struct.
3. Set the ovsrcu pointer
4. Call ovsrcu_synchronize to that all threads see that the policer is locked 
(Stop threads from accessing the meter)
5. Destroy the ingress policer.
6. Unlock the spinlock - if the spinlock is embedded in the ingress policer 
struct we have a problem here as it cannot  be free now, the struct has been 
destroyed.
7. Set the ovsrcu pointer
8. Synchronize again? (So that threads can see the ingress policer point for 
the netdev is now null)

Thanks
Ian

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to