On Sun, Sep 6, 2020 at 5:52 PM Eli Britstein <[email protected]> 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 <[email protected]>
> > Reviewed-by: Gaetan Rivet <[email protected]>
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
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
