On 11.08.2015 05:47, Alex Wang wrote:
> When dpdk configuration changes, all pmd threads are recreated
> and rx queues of each port are reloaded. After this process,
> rx queue could be mapped to a different pmd thread other than
> the one before reconfiguration. However, this is totally
> transparent to ofproto layer modules. So, if the ofproto-dpif-upcall
> module still holds ukeys generated before pmd thread recreation,
> this old ukey will collide with the ukey for the new upcalls
> from same traffic flow, causing flow installation failure.
>
> To fix the bug, this commit adds a new call-back function
> in dpif layer for notifying upper layer the purging of datapath
> (e.g. pmd threads recreation in dpif-netdev). So, the
> ofproto-dpif-upcall module can react properly with deleting
> all the ukeys and with collecting all flows' last stats.
>
> Reported-by: Ilya Maximets <i.maxim...@samsung.com>
> Signed-off-by: Alex Wang <al...@nicira.com>
> ---
> lib/dpif-netdev.c | 25 +++++++++++++++++++++++++
> lib/dpif-netlink.c | 1 +
> lib/dpif-provider.h | 11 ++++++++++-
> lib/dpif.c | 8 ++++++++
> lib/dpif.h | 12 ++++++++++++
> ofproto/ofproto-dpif-upcall.c | 16 ++++++++++++++++
> 6 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index c144352..a54853f 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -203,6 +203,11 @@ struct dp_netdev {
> upcall_callback *upcall_cb; /* Callback function for executing upcalls.
> */
> void *upcall_aux;
>
> + /* Callback function for notifying the purging of dp flows (during
> + * reseting pmd threads). */
> + dp_purge_callback *dp_purge_cb;
> + void *dp_purge_aux;
> +
> /* Stores all 'struct dp_netdev_pmd_thread's. */
> struct cmap poll_threads;
>
> @@ -223,6 +228,8 @@ struct dp_netdev {
>
> static struct dp_netdev_port *dp_netdev_lookup_port(const struct dp_netdev
> *dp,
> odp_port_t);
> +static void dp_netdev_disable_upcall(struct dp_netdev *);
> +static void dp_netdev_enable_upcall(struct dp_netdev *);
>
> enum dp_stat_type {
> DP_STAT_EXACT_HIT, /* Packets that had an exact match (emc). */
> @@ -2440,10 +2447,18 @@ dpif_netdev_pmd_set(struct dpif *dpif, unsigned int
> n_rxqs, const char *cmask)
> free(dp->pmd_cmask);
> dp->pmd_cmask = cmask ? xstrdup(cmask) : NULL;
>
> + /* Disables upcall and notifies higher layer about the incoming
> + * datapath purging. */
> + dp_netdev_disable_upcall(dp);
> + dp->dp_purge_cb(dp->dp_purge_aux);
> +
> /* Restores the non-pmd. */
> dp_netdev_set_nonpmd(dp);
> /* Restores all pmd threads. */
> dp_netdev_reset_pmd_threads(dp);
> +
> + /* Enables upcall after pmd reconfiguration. */
> + dp_netdev_enable_upcall(dp);
> }
It is better to enable upcalls before restoring PMD threads to prevent dropping
of packets.
>
> return 0;
> @@ -3419,6 +3434,15 @@ struct dp_netdev_execute_aux {
> };
>
> static void
> +dpif_netdev_register_dp_purge_cb(struct dpif *dpif, dp_purge_callback *cb,
> + void *aux)
> +{
> + struct dp_netdev *dp = get_dp_netdev(dpif);
> + dp->dp_purge_aux = aux;
> + dp->dp_purge_cb = cb;
> +}
> +
> +static void
> dpif_netdev_register_upcall_cb(struct dpif *dpif, upcall_callback *cb,
> void *aux)
> {
> @@ -3665,6 +3689,7 @@ const struct dpif_class dpif_netdev_class = {
> NULL, /* recv */
> NULL, /* recv_wait */
> NULL, /* recv_purge */
> + dpif_netdev_register_dp_purge_cb,
> dpif_netdev_register_upcall_cb,
> dpif_netdev_enable_upcall,
> dpif_netdev_disable_upcall,
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 8884a9f..f8f3092 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -2314,6 +2314,7 @@ const struct dpif_class dpif_netlink_class = {
> dpif_netlink_recv,
> dpif_netlink_recv_wait,
> dpif_netlink_recv_purge,
> + NULL, /* register_dp_purge_cb */
> NULL, /* register_upcall_cb */
> NULL, /* enable_upcall */
> NULL, /* disable_upcall */
> diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> index 28ea86f..8f2690f 100644
> --- a/lib/dpif-provider.h
> +++ b/lib/dpif-provider.h
> @@ -361,13 +361,22 @@ struct dpif_class {
> * return. */
> void (*recv_purge)(struct dpif *dpif);
>
> + /* When 'dpif' is about to purge the datapath, the higher layer may want
> + * to be notified so that it could try reacting accordingly (e.g.
> grabbing
> + * all flow stats before they are gone).
> + *
> + * Registers an upcall callback function with 'dpif'. This is only used
> if
double 'if' here too.
> + * if 'dpif' needs to notify the purging of datapath. 'aux' is passed to
> + * the callback on invocation. */
> + void (*register_dp_purge_cb)(struct dpif *, dp_purge_callback *, void
> *aux);
> +
> /* For datapaths that run in userspace (i.e. dpif-netdev), threads
> polling
> * for incoming packets can directly call upcall functions instead of
> * offloading packet processing to separate handler threads. Datapaths
> * that directly call upcall functions should use the functions below to
> * to register an upcall function and enable / disable upcalls.
> *
> - * Registers an upcall callback function with 'dpif'. This is only used
> if
> + * Registers an upcall callback function with 'dpif'. This is only used
> * if 'dpif' directly executes upcall functions. 'aux' is passed to the
> * callback on invocation. */
> void (*register_upcall_cb)(struct dpif *, upcall_callback *, void *aux);
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 6e26f30..9a67a24 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1346,6 +1346,14 @@ dpif_handlers_set(struct dpif *dpif, uint32_t
> n_handlers)
> }
>
> void
> +dpif_register_dp_purge_cb(struct dpif *dpif, dp_purge_callback *cb, void
> *aux)
> +{
> + if (dpif->dpif_class->register_dp_purge_cb) {
> + dpif->dpif_class->register_dp_purge_cb(dpif, cb, aux);
> + }
> +}
> +
> +void
> dpif_register_upcall_cb(struct dpif *dpif, upcall_callback *cb, void *aux)
> {
> if (dpif->dpif_class->register_upcall_cb) {
> diff --git a/lib/dpif.h b/lib/dpif.h
> index ea9caf8..dca5d00 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -787,6 +787,18 @@ struct dpif_upcall {
> struct nlattr *actions; /* Argument to OVS_ACTION_ATTR_USERSPACE. */
> };
>
> +/* A callback to notify higher layer of dpif about to be purged, so that
> + * higher layer could try reacting to this (e.g. grabbing all flow stats
> + * before they are gone). This function is currently implemented only by
> + * dpif-netdev.
> + *
> + * The caller only needs to provide the 'aux' pointer passed down by higher
> + * layer from the dpif_register_notify_cb() function.
> + */
> +typedef void dp_purge_callback(void *aux);
> +
> +void dpif_register_dp_purge_cb(struct dpif *, dp_purge_callback *, void
> *aux);
> +
> /* A callback to process an upcall, currently implemented only by
> dpif-netdev.
> *
> * The caller provides the 'packet' and 'flow' to process, the corresponding
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 0f2e186..6385abc 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -306,6 +306,7 @@ static int upcall_receive(struct upcall *, const struct
> dpif_backer *,
> static void upcall_uninit(struct upcall *);
>
> static upcall_callback upcall_cb;
> +static dp_purge_callback dp_purge_cb;
>
> static atomic_bool enable_megaflows = ATOMIC_VAR_INIT(true);
> static atomic_bool enable_ufid = ATOMIC_VAR_INIT(true);
> @@ -358,6 +359,7 @@ udpif_create(struct dpif_backer *backer, struct dpif
> *dpif)
> }
>
> dpif_register_upcall_cb(dpif, upcall_cb, udpif);
> + dpif_register_dp_purge_cb(dpif, dp_purge_cb, udpif);
>
> return udpif;
> }
> @@ -1110,6 +1112,20 @@ out:
> return error;
> }
>
> +/* In reaction to dpif purge, purges all 'ukey's. */
> +static void
> +dp_purge_cb(void *aux)
> +{
> + struct udpif *udpif = aux;
> + size_t i;
> +
> + for (i = 0; i < udpif->n_revalidators; i++) {
> + struct revalidator *revalidator = &udpif->revalidators[i];
> +
> + revalidator_purge(revalidator);
> + }
> +}
> +
> static int
> process_upcall(struct udpif *udpif, struct upcall *upcall,
> struct ofpbuf *odp_actions, struct flow_wildcards *wc)
>
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
