| Message ID | 20200518192731.30804-2-sriharsha.basavapatna@broadcom.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RFC for Partial action offload | expand |
On 5/18/2020 10:27 PM, Sriharsha Basavapatna wrote: > This patch refactors dp_netdev_flow_offload_put() to prepare for > more changes to support partial action offload, in subsequent patches. > > - Add a wrapper function to allocate flow-mark. > - Move netdev_ports_get() to before flow-mark allocation. Moving netdev_ports_get position is a change of logic, not just refactor. It should be in a separate commit. > > Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> > --- > lib/dpif-netdev.c | 71 ++++++++++++++++++++++++++++++----------------- > 1 file changed, 45 insertions(+), 26 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index ea7b187b2..781b233f4 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -2357,6 +2357,43 @@ dp_netdev_flow_offload_del(struct dp_flow_offload_item *offload) > return mark_to_flow_disassociate(offload->pmd, offload->flow); > } > > +static int > +dp_netdev_alloc_flow_mark(struct dp_netdev_flow *flow, bool modification, > + uint32_t *markp) > +{ > + uint32_t mark; > + > + if (modification) { > + mark = flow->mark; > + ovs_assert(mark != INVALID_FLOW_MARK); > + *markp = mark; > + return 0; > + } > + > + /* > + * If a mega flow has already been offloaded (from other PMD > + * instances), do not offload it again. > + */ > + mark = megaflow_to_mark_find(&flow->mega_ufid); > + if (mark != INVALID_FLOW_MARK) { > + VLOG_DBG("Flow has already been offloaded with mark %u\n", mark); > + if (flow->mark != INVALID_FLOW_MARK) { > + ovs_assert(flow->mark == mark); > + } else { > + mark_to_flow_associate(mark, flow); > + } > + return 1; > + } > + > + mark = netdev_offload_flow_mark_alloc(); > + if (mark == INVALID_FLOW_MARK) { > + VLOG_ERR("Failed to allocate flow mark!\n"); > + } > + > + *markp = mark; > + return 0; > +} > + > /* > * There are two flow offload operations here: addition and modification. > * > @@ -2385,36 +2422,18 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item *offload) > return -1; > } > > - if (modification) { > - mark = flow->mark; > - ovs_assert(mark != INVALID_FLOW_MARK); > - } else { > - /* > - * If a mega flow has already been offloaded (from other PMD > - * instances), do not offload it again. > - */ > - mark = megaflow_to_mark_find(&flow->mega_ufid); > - if (mark != INVALID_FLOW_MARK) { > - VLOG_DBG("Flow has already been offloaded with mark %u\n", mark); > - if (flow->mark != INVALID_FLOW_MARK) { > - ovs_assert(flow->mark == mark); > - } else { > - mark_to_flow_associate(mark, flow); > - } > - return 0; > - } > + port = netdev_ports_get(in_port, dpif_type_str); > + if (!port) { > + return -1; > + } > > - mark = netdev_offload_flow_mark_alloc(); > - if (mark == INVALID_FLOW_MARK) { > - VLOG_ERR("Failed to allocate flow mark!\n"); > - } > + if (dp_netdev_alloc_flow_mark(flow, modification, &mark)) { > + /* flow already offloaded */ > + netdev_close(port); > + return 0; > } > info.flow_mark = mark; > > - port = netdev_ports_get(in_port, dpif_type_str); > - if (!port) { > - goto err_free; > - } > /* Taking a global 'port_mutex' to fulfill thread safety restrictions for > * the netdev-offload-dpdk module. */ > ovs_mutex_lock(&pmd->dp->port_mutex);
On Tue, May 26, 2020 at 7:12 PM Eli Britstein <elibr@mellanox.com> wrote: > > > On 5/18/2020 10:27 PM, Sriharsha Basavapatna wrote: > > This patch refactors dp_netdev_flow_offload_put() to prepare for > > more changes to support partial action offload, in subsequent patches. > > > > - Add a wrapper function to allocate flow-mark. > > - Move netdev_ports_get() to before flow-mark allocation. > Moving netdev_ports_get position is a change of logic, not just > refactor. It should be in a separate commit. I don't want to create 2 commits for this. This is just refactoring, since 1) it doesn't change the functionality of existing code (we still allocate a mark if required and we still get a reference to the in-port's netdev) 2) nor does it change the interface (inputs/outputs to dp_netdev_flow_offload_put() remain the same) . But to make it more clear, I'll add additional comments in the commit log. Thanks, -Harsha > > > > Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> > > --- > > lib/dpif-netdev.c | 71 ++++++++++++++++++++++++++++++----------------- > > 1 file changed, 45 insertions(+), 26 deletions(-) > > > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > > index ea7b187b2..781b233f4 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -2357,6 +2357,43 @@ dp_netdev_flow_offload_del(struct dp_flow_offload_item *offload) > > return mark_to_flow_disassociate(offload->pmd, offload->flow); > > } > > > > +static int > > +dp_netdev_alloc_flow_mark(struct dp_netdev_flow *flow, bool modification, > > + uint32_t *markp) > > +{ > > + uint32_t mark; > > + > > + if (modification) { > > + mark = flow->mark; > > + ovs_assert(mark != INVALID_FLOW_MARK); > > + *markp = mark; > > + return 0; > > + } > > + > > + /* > > + * If a mega flow has already been offloaded (from other PMD > > + * instances), do not offload it again. > > + */ > > + mark = megaflow_to_mark_find(&flow->mega_ufid); > > + if (mark != INVALID_FLOW_MARK) { > > + VLOG_DBG("Flow has already been offloaded with mark %u\n", mark); > > + if (flow->mark != INVALID_FLOW_MARK) { > > + ovs_assert(flow->mark == mark); > > + } else { > > + mark_to_flow_associate(mark, flow); > > + } > > + return 1; > > + } > > + > > + mark = netdev_offload_flow_mark_alloc(); > > + if (mark == INVALID_FLOW_MARK) { > > + VLOG_ERR("Failed to allocate flow mark!\n"); > > + } > > + > > + *markp = mark; > > + return 0; > > +} > > + > > /* > > * There are two flow offload operations here: addition and modification. > > * > > @@ -2385,36 +2422,18 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item *offload) > > return -1; > > } > > > > - if (modification) { > > - mark = flow->mark; > > - ovs_assert(mark != INVALID_FLOW_MARK); > > - } else { > > - /* > > - * If a mega flow has already been offloaded (from other PMD > > - * instances), do not offload it again. > > - */ > > - mark = megaflow_to_mark_find(&flow->mega_ufid); > > - if (mark != INVALID_FLOW_MARK) { > > - VLOG_DBG("Flow has already been offloaded with mark %u\n", mark); > > - if (flow->mark != INVALID_FLOW_MARK) { > > - ovs_assert(flow->mark == mark); > > - } else { > > - mark_to_flow_associate(mark, flow); > > - } > > - return 0; > > - } > > + port = netdev_ports_get(in_port, dpif_type_str); > > + if (!port) { > > + return -1; > > + } > > > > - mark = netdev_offload_flow_mark_alloc(); > > - if (mark == INVALID_FLOW_MARK) { > > - VLOG_ERR("Failed to allocate flow mark!\n"); > > - } > > + if (dp_netdev_alloc_flow_mark(flow, modification, &mark)) { > > + /* flow already offloaded */ > > + netdev_close(port); > > + return 0; > > } > > info.flow_mark = mark; > > > > - port = netdev_ports_get(in_port, dpif_type_str); > > - if (!port) { > > - goto err_free; > > - } > > /* Taking a global 'port_mutex' to fulfill thread safety restrictions for > > * the netdev-offload-dpdk module. */ > > ovs_mutex_lock(&pmd->dp->port_mutex);
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index ea7b187b2..781b233f4 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -2357,6 +2357,43 @@ dp_netdev_flow_offload_del(struct dp_flow_offload_item *offload) return mark_to_flow_disassociate(offload->pmd, offload->flow); } +static int +dp_netdev_alloc_flow_mark(struct dp_netdev_flow *flow, bool modification, + uint32_t *markp) +{ + uint32_t mark; + + if (modification) { + mark = flow->mark; + ovs_assert(mark != INVALID_FLOW_MARK); + *markp = mark; + return 0; + } + + /* + * If a mega flow has already been offloaded (from other PMD + * instances), do not offload it again. + */ + mark = megaflow_to_mark_find(&flow->mega_ufid); + if (mark != INVALID_FLOW_MARK) { + VLOG_DBG("Flow has already been offloaded with mark %u\n", mark); + if (flow->mark != INVALID_FLOW_MARK) { + ovs_assert(flow->mark == mark); + } else { + mark_to_flow_associate(mark, flow); + } + return 1; + } + + mark = netdev_offload_flow_mark_alloc(); + if (mark == INVALID_FLOW_MARK) { + VLOG_ERR("Failed to allocate flow mark!\n"); + } + + *markp = mark; + return 0; +} + /* * There are two flow offload operations here: addition and modification. * @@ -2385,36 +2422,18 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item *offload) return -1; } - if (modification) { - mark = flow->mark; - ovs_assert(mark != INVALID_FLOW_MARK); - } else { - /* - * If a mega flow has already been offloaded (from other PMD - * instances), do not offload it again. - */ - mark = megaflow_to_mark_find(&flow->mega_ufid); - if (mark != INVALID_FLOW_MARK) { - VLOG_DBG("Flow has already been offloaded with mark %u\n", mark); - if (flow->mark != INVALID_FLOW_MARK) { - ovs_assert(flow->mark == mark); - } else { - mark_to_flow_associate(mark, flow); - } - return 0; - } + port = netdev_ports_get(in_port, dpif_type_str); + if (!port) { + return -1; + } - mark = netdev_offload_flow_mark_alloc(); - if (mark == INVALID_FLOW_MARK) { - VLOG_ERR("Failed to allocate flow mark!\n"); - } + if (dp_netdev_alloc_flow_mark(flow, modification, &mark)) { + /* flow already offloaded */ + netdev_close(port); + return 0; } info.flow_mark = mark; - port = netdev_ports_get(in_port, dpif_type_str); - if (!port) { - goto err_free; - } /* Taking a global 'port_mutex' to fulfill thread safety restrictions for * the netdev-offload-dpdk module. */ ovs_mutex_lock(&pmd->dp->port_mutex);
This patch refactors dp_netdev_flow_offload_put() to prepare for more changes to support partial action offload, in subsequent patches. - Add a wrapper function to allocate flow-mark. - Move netdev_ports_get() to before flow-mark allocation. Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> --- lib/dpif-netdev.c | 71 ++++++++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 26 deletions(-)