Hi Ian,
Thursday, March 15, 2018 1:55 PM, Stokes, Ian:
> Adding Shahaf Shuler
> > > +static void
> > > +try_netdev_flow_put(struct dp_netdev_pmd_thread *pmd,
> odp_port_t
> > in_port,
> > > + struct dp_netdev_flow *flow, struct match *match,
> > > + const struct nlattr *actions, size_t actions_len) {
> > > + struct offload_info info;
> > > + struct dp_netdev_port *port;
> > > + bool modification = flow->mark != INVALID_FLOW_MARK;
> > > + const char *op = modification ? "modify" : "add";
> > > + uint32_t mark;
> > > + int ret;
> > > +
> > > + ovs_mutex_lock(&flow_mark.mutex);
> > > +
> > > + if (modification) {
> > > + mark = flow->mark;
> > > + } else {
> > > + if (!netdev_is_flow_api_enabled()) {
> > > + goto out;
> > > + }
> > > +
> > > + /*
> > > + * 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);
> > > + mark_to_flow_associate(mark, flow);
> > > + goto out;
> > > + }
> > > +
> > > + mark = flow_mark_alloc();
> > > + if (mark == INVALID_FLOW_MARK) {
> > > + VLOG_ERR("failed to allocate flow mark!\n");
> > > + goto out;
> > > + }
> > > + }
> > > + info.flow_mark = mark;
> > > +
> > > + ovs_mutex_lock(&pmd->dp->port_mutex);
> > > + port = dp_netdev_lookup_port(pmd->dp, in_port);
> > > + if (!port) {
> > > + ovs_mutex_unlock(&pmd->dp->port_mutex);
> > > + goto out;
> > > + }
> > > + ret = netdev_flow_put(port->netdev, match,
> > > + CONST_CAST(struct nlattr *, actions),
> > > + actions_len, &flow->mega_ufid, &info, NULL);
> > > + ovs_mutex_unlock(&pmd->dp->port_mutex);
> > > +
> > > + if (ret) {
> > > + VLOG_ERR("failed to %s netdev flow with mark %u\n", op, mark);
> > > + if (!modification) {
> > > + flow_mark_free(mark);
> > > + } else {
> > > + mark_to_flow_disassociate(pmd, flow);
> >
> > In the case here would it not be a call to flush? If
> > the flow could not be modified wouldn't you have to remove all
> > references for it not just on this PMD?
I discussed it with Finn,
Let's first agree on the case were flow modification succeeds. Please correct
me if I am wrong about the flow here. For flow modification each PMD will
attempt to modify the flow on its private flow table.
This will result in multiple flow destroy and creation on the device
(netdev_flow_put). After the last PMD finishes each PMD has the modified flow
on its private table and the device has one flow with a single mark which was
created by the last PMD.
If such behavior is OK, then the error case should also be OK. In case of flow
modification error, each PMD will disassociate the flow from its private table.
The last PMD which has the last reference for the flow mark will remove it also
from the device. Hence there is no need to flush the flow from each PMD upon
the first failure.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
