| Message ID | 20200730135237.22157-1-elibr@mellanox.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,1/1] netdev-offload-dpdk: Preserve HW statistics for modified flows | expand |
ping On 7/30/2020 4:52 PM, Eli Britstein wrote: > In case of a flow modification, preserve the HW statistics of the old HW > flow to the new one. > > Signed-off-by: Eli Britstein <elibr@mellanox.com> > Reviewed-by: Gaetan Rivet <gaetanr@mellanox.com> > --- > lib/netdev-offload-dpdk.c | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > index de6101e4d..960acb2da 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -78,7 +78,7 @@ ufid_to_rte_flow_data_find(const ovs_u128 *ufid) > return NULL; > } > > -static inline void > +static inline struct ufid_to_rte_flow_data * > ufid_to_rte_flow_associate(const ovs_u128 *ufid, > struct rte_flow *rte_flow, bool actions_offloaded) > { > @@ -103,6 +103,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, > > cmap_insert(&ufid_to_rte_flow, > CONST_CAST(struct cmap_node *, &data->node), hash); > + return data; > } > > static inline void > @@ -1407,7 +1408,7 @@ out: > return flow; > } > > -static int > +static struct ufid_to_rte_flow_data * > netdev_offload_dpdk_add_flow(struct netdev *netdev, > struct match *match, > struct nlattr *nl_actions, > @@ -1416,6 +1417,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, > struct offload_info *info) > { > struct flow_patterns patterns = { .items = NULL, .cnt = 0 }; > + struct ufid_to_rte_flow_data *flows_data = NULL; > bool actions_offloaded = true; > struct rte_flow *flow; > int ret = 0; > @@ -1442,13 +1444,13 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, > ret = -1; > goto out; > } > - ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); > + flows_data = ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); > VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT, > netdev_get_name(netdev), flow, UUID_ARGS((struct uuid *)ufid)); > > out: > free_flow_patterns(&patterns); > - return ret; > + return flows_data; > } > > static int > @@ -1482,14 +1484,19 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, > struct dpif_flow_stats *stats) > { > struct ufid_to_rte_flow_data *rte_flow_data; > + struct dpif_flow_stats old_stats; > + bool modification = false; > int ret; > > /* > * If an old rte_flow exists, it means it's a flow modification. > * Here destroy the old rte flow first before adding a new one. > + * Keep the stats for the newly created rule. > */ > rte_flow_data = ufid_to_rte_flow_data_find(ufid); > if (rte_flow_data && rte_flow_data->rte_flow) { > + old_stats = rte_flow_data->stats; > + modification = true; > ret = netdev_offload_dpdk_destroy_flow(netdev, ufid, > rte_flow_data->rte_flow); > if (ret < 0) { > @@ -1497,11 +1504,18 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, > } > } > > + rte_flow_data = netdev_offload_dpdk_add_flow(netdev, match, actions, > + actions_len, ufid, info); > + if (!rte_flow_data) { > + return -1; > + } > + if (modification) { > + rte_flow_data->stats = old_stats; > + } > if (stats) { > - memset(stats, 0, sizeof *stats); > + *stats = rte_flow_data->stats; > } > - return netdev_offload_dpdk_add_flow(netdev, match, actions, > - actions_len, ufid, info); > + return 0; > } > > static int
ping On 7/30/2020 4:52 PM, Eli Britstein wrote: > In case of a flow modification, preserve the HW statistics of the old HW > flow to the new one. > > Signed-off-by: Eli Britstein <elibr@mellanox.com> > Reviewed-by: Gaetan Rivet <gaetanr@mellanox.com> > --- > lib/netdev-offload-dpdk.c | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > index de6101e4d..960acb2da 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -78,7 +78,7 @@ ufid_to_rte_flow_data_find(const ovs_u128 *ufid) > return NULL; > } > > -static inline void > +static inline struct ufid_to_rte_flow_data * > ufid_to_rte_flow_associate(const ovs_u128 *ufid, > struct rte_flow *rte_flow, bool actions_offloaded) > { > @@ -103,6 +103,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, > > cmap_insert(&ufid_to_rte_flow, > CONST_CAST(struct cmap_node *, &data->node), hash); > + return data; > } > > static inline void > @@ -1407,7 +1408,7 @@ out: > return flow; > } > > -static int > +static struct ufid_to_rte_flow_data * > netdev_offload_dpdk_add_flow(struct netdev *netdev, > struct match *match, > struct nlattr *nl_actions, > @@ -1416,6 +1417,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, > struct offload_info *info) > { > struct flow_patterns patterns = { .items = NULL, .cnt = 0 }; > + struct ufid_to_rte_flow_data *flows_data = NULL; > bool actions_offloaded = true; > struct rte_flow *flow; > int ret = 0; > @@ -1442,13 +1444,13 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, > ret = -1; > goto out; > } > - ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); > + flows_data = ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); > VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT, > netdev_get_name(netdev), flow, UUID_ARGS((struct uuid *)ufid)); > > out: > free_flow_patterns(&patterns); > - return ret; > + return flows_data; > } > > static int > @@ -1482,14 +1484,19 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, > struct dpif_flow_stats *stats) > { > struct ufid_to_rte_flow_data *rte_flow_data; > + struct dpif_flow_stats old_stats; > + bool modification = false; > int ret; > > /* > * If an old rte_flow exists, it means it's a flow modification. > * Here destroy the old rte flow first before adding a new one. > + * Keep the stats for the newly created rule. > */ > rte_flow_data = ufid_to_rte_flow_data_find(ufid); > if (rte_flow_data && rte_flow_data->rte_flow) { > + old_stats = rte_flow_data->stats; > + modification = true; > ret = netdev_offload_dpdk_destroy_flow(netdev, ufid, > rte_flow_data->rte_flow); > if (ret < 0) { > @@ -1497,11 +1504,18 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, > } > } > > + rte_flow_data = netdev_offload_dpdk_add_flow(netdev, match, actions, > + actions_len, ufid, info); > + if (!rte_flow_data) { > + return -1; > + } > + if (modification) { > + rte_flow_data->stats = old_stats; > + } > if (stats) { > - memset(stats, 0, sizeof *stats); > + *stats = rte_flow_data->stats; > } > - return netdev_offload_dpdk_add_flow(netdev, match, actions, > - actions_len, ufid, info); > + return 0; > } > > static int
On Sun, Sep 6, 2020 at 5:52 PM Eli Britstein <elibr@nvidia.com> wrote: > > ping > > On 7/30/2020 4:52 PM, Eli Britstein wrote: > > In case of a flow modification, preserve the HW statistics of the old HW > > flow to the new one. > > > > Signed-off-by: Eli Britstein <elibr@mellanox.com> > > Reviewed-by: Gaetan Rivet <gaetanr@mellanox.com> Update fixes: tag > > --- > > lib/netdev-offload-dpdk.c | 28 +++++++++++++++++++++------- > > 1 file changed, 21 insertions(+), 7 deletions(-) > > > > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > > index de6101e4d..960acb2da 100644 > > --- a/lib/netdev-offload-dpdk.c > > +++ b/lib/netdev-offload-dpdk.c > > @@ -78,7 +78,7 @@ ufid_to_rte_flow_data_find(const ovs_u128 *ufid) > > return NULL; > > } > > > > -static inline void > > +static inline struct ufid_to_rte_flow_data * > > ufid_to_rte_flow_associate(const ovs_u128 *ufid, > > struct rte_flow *rte_flow, bool actions_offloaded) > > { > > @@ -103,6 +103,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, > > > > cmap_insert(&ufid_to_rte_flow, > > CONST_CAST(struct cmap_node *, &data->node), hash); > > + return data; > > } > > > > static inline void > > @@ -1407,7 +1408,7 @@ out: > > return flow; > > } > > > > -static int > > +static struct ufid_to_rte_flow_data * > > netdev_offload_dpdk_add_flow(struct netdev *netdev, > > struct match *match, > > struct nlattr *nl_actions, > > @@ -1416,6 +1417,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, > > struct offload_info *info) > > { > > struct flow_patterns patterns = { .items = NULL, .cnt = 0 }; > > + struct ufid_to_rte_flow_data *flows_data = NULL; > > bool actions_offloaded = true; > > struct rte_flow *flow; > > int ret = 0; We can eliminate 'ret' with this change, since it is only being used to catch the return value of parse_flow_match(). > > @@ -1442,13 +1444,13 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, > > ret = -1; Relates to the above comment. > > goto out; > > } > > - ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); > > + flows_data = ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); > > VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT, > > netdev_get_name(netdev), flow, UUID_ARGS((struct uuid *)ufid)); > > > > out: > > free_flow_patterns(&patterns); > > - return ret; > > + return flows_data; > > } > > > > static int > > @@ -1482,14 +1484,19 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, > > struct dpif_flow_stats *stats) > > { > > struct ufid_to_rte_flow_data *rte_flow_data; > > + struct dpif_flow_stats old_stats; > > + bool modification = false; > > int ret; > > > > /* > > * If an old rte_flow exists, it means it's a flow modification. > > * Here destroy the old rte flow first before adding a new one. > > + * Keep the stats for the newly created rule. > > */ > > rte_flow_data = ufid_to_rte_flow_data_find(ufid); > > if (rte_flow_data && rte_flow_data->rte_flow) { > > + old_stats = rte_flow_data->stats; > > + modification = true; > > ret = netdev_offload_dpdk_destroy_flow(netdev, ufid, > > rte_flow_data->rte_flow); > > if (ret < 0) { > > @@ -1497,11 +1504,18 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, > > } > > } > > > > + rte_flow_data = netdev_offload_dpdk_add_flow(netdev, match, actions, > > + actions_len, ufid, info); > > + if (!rte_flow_data) { > > + return -1; > > + } > > + if (modification) { > > + rte_flow_data->stats = old_stats; > > + } > > if (stats) { > > - memset(stats, 0, sizeof *stats); What if it is a new flow, don't we need to clear the stats ? > > + *stats = rte_flow_data->stats; > > } > > - return netdev_offload_dpdk_add_flow(netdev, match, actions, > > - actions_len, ufid, info); > > + return 0; > > } > > > > static int > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 10/12/2020 11:45 AM, Sriharsha Basavapatna wrote: > On Sun, Sep 6, 2020 at 5:52 PM Eli Britstein <elibr@nvidia.com> wrote: >> ping >> >> On 7/30/2020 4:52 PM, Eli Britstein wrote: >>> In case of a flow modification, preserve the HW statistics of the old HW >>> flow to the new one. >>> >>> Signed-off-by: Eli Britstein <elibr@mellanox.com> >>> Reviewed-by: Gaetan Rivet <gaetanr@mellanox.com> > Update fixes: tag I'll put 3c7330ebf036 netdev-offload-dpdk: Support offload of output action. As it's the first commit that does full offload. Before that there are no HW rules that count. What do you think? >>> --- >>> lib/netdev-offload-dpdk.c | 28 +++++++++++++++++++++------- >>> 1 file changed, 21 insertions(+), 7 deletions(-) >>> >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >>> index de6101e4d..960acb2da 100644 >>> --- a/lib/netdev-offload-dpdk.c >>> +++ b/lib/netdev-offload-dpdk.c >>> @@ -78,7 +78,7 @@ ufid_to_rte_flow_data_find(const ovs_u128 *ufid) >>> return NULL; >>> } >>> >>> -static inline void >>> +static inline struct ufid_to_rte_flow_data * >>> ufid_to_rte_flow_associate(const ovs_u128 *ufid, >>> struct rte_flow *rte_flow, bool actions_offloaded) >>> { >>> @@ -103,6 +103,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, >>> >>> cmap_insert(&ufid_to_rte_flow, >>> CONST_CAST(struct cmap_node *, &data->node), hash); >>> + return data; >>> } >>> >>> static inline void >>> @@ -1407,7 +1408,7 @@ out: >>> return flow; >>> } >>> >>> -static int >>> +static struct ufid_to_rte_flow_data * >>> netdev_offload_dpdk_add_flow(struct netdev *netdev, >>> struct match *match, >>> struct nlattr *nl_actions, >>> @@ -1416,6 +1417,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, >>> struct offload_info *info) >>> { >>> struct flow_patterns patterns = { .items = NULL, .cnt = 0 }; >>> + struct ufid_to_rte_flow_data *flows_data = NULL; >>> bool actions_offloaded = true; >>> struct rte_flow *flow; >>> int ret = 0; > We can eliminate 'ret' with this change, since it is only being used > to catch the return value of parse_flow_match(). Ack >>> @@ -1442,13 +1444,13 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, >>> ret = -1; > Relates to the above comment. >>> goto out; >>> } >>> - ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); >>> + flows_data = ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); >>> VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT, >>> netdev_get_name(netdev), flow, UUID_ARGS((struct uuid *)ufid)); >>> >>> out: >>> free_flow_patterns(&patterns); >>> - return ret; >>> + return flows_data; >>> } >>> >>> static int >>> @@ -1482,14 +1484,19 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, >>> struct dpif_flow_stats *stats) >>> { >>> struct ufid_to_rte_flow_data *rte_flow_data; >>> + struct dpif_flow_stats old_stats; >>> + bool modification = false; >>> int ret; >>> >>> /* >>> * If an old rte_flow exists, it means it's a flow modification. >>> * Here destroy the old rte flow first before adding a new one. >>> + * Keep the stats for the newly created rule. >>> */ >>> rte_flow_data = ufid_to_rte_flow_data_find(ufid); >>> if (rte_flow_data && rte_flow_data->rte_flow) { >>> + old_stats = rte_flow_data->stats; >>> + modification = true; >>> ret = netdev_offload_dpdk_destroy_flow(netdev, ufid, >>> rte_flow_data->rte_flow); >>> if (ret < 0) { >>> @@ -1497,11 +1504,18 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, >>> } >>> } >>> >>> + rte_flow_data = netdev_offload_dpdk_add_flow(netdev, match, actions, >>> + actions_len, ufid, info); >>> + if (!rte_flow_data) { >>> + return -1; >>> + } >>> + if (modification) { >>> + rte_flow_data->stats = old_stats; >>> + } >>> if (stats) { >>> - memset(stats, 0, sizeof *stats); > What if it is a new flow, don't we need to clear the stats ? It is already cleared before. Allocation is xZalloc. See in ufid_to_rte_flow_associate: struct ufid_to_rte_flow_data *data = xzalloc(sizeof *data); >>> + *stats = rte_flow_data->stats; >>> } >>> - return netdev_offload_dpdk_add_flow(netdev, match, actions, >>> - actions_len, ufid, info); >>> + return 0; >>> } >>> >>> static int >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Mon, Oct 12, 2020 at 4:43 PM Eli Britstein <elibr@nvidia.com> wrote: > > > On 10/12/2020 11:45 AM, Sriharsha Basavapatna wrote: > > On Sun, Sep 6, 2020 at 5:52 PM Eli Britstein <elibr@nvidia.com> wrote: > >> ping > >> > >> On 7/30/2020 4:52 PM, Eli Britstein wrote: > >>> In case of a flow modification, preserve the HW statistics of the old HW > >>> flow to the new one. > >>> > >>> Signed-off-by: Eli Britstein <elibr@mellanox.com> > >>> Reviewed-by: Gaetan Rivet <gaetanr@mellanox.com> > > Update fixes: tag > I'll put 3c7330ebf036 netdev-offload-dpdk: Support offload of output action. > As it's the first commit that does full offload. Before that there are > no HW rules that count. What do you think? That's fine. > >>> --- > >>> lib/netdev-offload-dpdk.c | 28 +++++++++++++++++++++------- > >>> 1 file changed, 21 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > >>> index de6101e4d..960acb2da 100644 > >>> --- a/lib/netdev-offload-dpdk.c > >>> +++ b/lib/netdev-offload-dpdk.c > >>> @@ -78,7 +78,7 @@ ufid_to_rte_flow_data_find(const ovs_u128 *ufid) > >>> return NULL; > >>> } > >>> > >>> -static inline void > >>> +static inline struct ufid_to_rte_flow_data * > >>> ufid_to_rte_flow_associate(const ovs_u128 *ufid, > >>> struct rte_flow *rte_flow, bool actions_offloaded) > >>> { > >>> @@ -103,6 +103,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, > >>> > >>> cmap_insert(&ufid_to_rte_flow, > >>> CONST_CAST(struct cmap_node *, &data->node), hash); > >>> + return data; > >>> } > >>> > >>> static inline void > >>> @@ -1407,7 +1408,7 @@ out: > >>> return flow; > >>> } > >>> > >>> -static int > >>> +static struct ufid_to_rte_flow_data * > >>> netdev_offload_dpdk_add_flow(struct netdev *netdev, > >>> struct match *match, > >>> struct nlattr *nl_actions, > >>> @@ -1416,6 +1417,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, > >>> struct offload_info *info) > >>> { > >>> struct flow_patterns patterns = { .items = NULL, .cnt = 0 }; > >>> + struct ufid_to_rte_flow_data *flows_data = NULL; > >>> bool actions_offloaded = true; > >>> struct rte_flow *flow; > >>> int ret = 0; > > We can eliminate 'ret' with this change, since it is only being used > > to catch the return value of parse_flow_match(). > Ack > >>> @@ -1442,13 +1444,13 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, > >>> ret = -1; > > Relates to the above comment. > >>> goto out; > >>> } > >>> - ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); > >>> + flows_data = ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); > >>> VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT, > >>> netdev_get_name(netdev), flow, UUID_ARGS((struct uuid *)ufid)); > >>> > >>> out: > >>> free_flow_patterns(&patterns); > >>> - return ret; > >>> + return flows_data; > >>> } > >>> > >>> static int > >>> @@ -1482,14 +1484,19 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, > >>> struct dpif_flow_stats *stats) > >>> { > >>> struct ufid_to_rte_flow_data *rte_flow_data; > >>> + struct dpif_flow_stats old_stats; > >>> + bool modification = false; > >>> int ret; > >>> > >>> /* > >>> * If an old rte_flow exists, it means it's a flow modification. > >>> * Here destroy the old rte flow first before adding a new one. > >>> + * Keep the stats for the newly created rule. > >>> */ > >>> rte_flow_data = ufid_to_rte_flow_data_find(ufid); > >>> if (rte_flow_data && rte_flow_data->rte_flow) { > >>> + old_stats = rte_flow_data->stats; > >>> + modification = true; > >>> ret = netdev_offload_dpdk_destroy_flow(netdev, ufid, > >>> rte_flow_data->rte_flow); > >>> if (ret < 0) { > >>> @@ -1497,11 +1504,18 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, > >>> } > >>> } > >>> > >>> + rte_flow_data = netdev_offload_dpdk_add_flow(netdev, match, actions, > >>> + actions_len, ufid, info); > >>> + if (!rte_flow_data) { > >>> + return -1; > >>> + } > >>> + if (modification) { > >>> + rte_flow_data->stats = old_stats; > >>> + } > >>> if (stats) { > >>> - memset(stats, 0, sizeof *stats); > > What if it is a new flow, don't we need to clear the stats ? > > It is already cleared before. Allocation is xZalloc. See in > ufid_to_rte_flow_associate: > > struct ufid_to_rte_flow_data *data = xzalloc(sizeof *data); Ok, thanks. > > >>> + *stats = rte_flow_data->stats; > >>> } > >>> - return netdev_offload_dpdk_add_flow(netdev, match, actions, > >>> - actions_len, ufid, info); > >>> + return 0; > >>> } > >>> > >>> static int > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index de6101e4d..960acb2da 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -78,7 +78,7 @@ ufid_to_rte_flow_data_find(const ovs_u128 *ufid) return NULL; } -static inline void +static inline struct ufid_to_rte_flow_data * ufid_to_rte_flow_associate(const ovs_u128 *ufid, struct rte_flow *rte_flow, bool actions_offloaded) { @@ -103,6 +103,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid, cmap_insert(&ufid_to_rte_flow, CONST_CAST(struct cmap_node *, &data->node), hash); + return data; } static inline void @@ -1407,7 +1408,7 @@ out: return flow; } -static int +static struct ufid_to_rte_flow_data * netdev_offload_dpdk_add_flow(struct netdev *netdev, struct match *match, struct nlattr *nl_actions, @@ -1416,6 +1417,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, struct offload_info *info) { struct flow_patterns patterns = { .items = NULL, .cnt = 0 }; + struct ufid_to_rte_flow_data *flows_data = NULL; bool actions_offloaded = true; struct rte_flow *flow; int ret = 0; @@ -1442,13 +1444,13 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, ret = -1; goto out; } - ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); + flows_data = ufid_to_rte_flow_associate(ufid, flow, actions_offloaded); VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT, netdev_get_name(netdev), flow, UUID_ARGS((struct uuid *)ufid)); out: free_flow_patterns(&patterns); - return ret; + return flows_data; } static int @@ -1482,14 +1484,19 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, struct dpif_flow_stats *stats) { struct ufid_to_rte_flow_data *rte_flow_data; + struct dpif_flow_stats old_stats; + bool modification = false; int ret; /* * If an old rte_flow exists, it means it's a flow modification. * Here destroy the old rte flow first before adding a new one. + * Keep the stats for the newly created rule. */ rte_flow_data = ufid_to_rte_flow_data_find(ufid); if (rte_flow_data && rte_flow_data->rte_flow) { + old_stats = rte_flow_data->stats; + modification = true; ret = netdev_offload_dpdk_destroy_flow(netdev, ufid, rte_flow_data->rte_flow); if (ret < 0) { @@ -1497,11 +1504,18 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, struct match *match, } } + rte_flow_data = netdev_offload_dpdk_add_flow(netdev, match, actions, + actions_len, ufid, info); + if (!rte_flow_data) { + return -1; + } + if (modification) { + rte_flow_data->stats = old_stats; + } if (stats) { - memset(stats, 0, sizeof *stats); + *stats = rte_flow_data->stats; } - return netdev_offload_dpdk_add_flow(netdev, match, actions, - actions_len, ufid, info); + return 0; } static int