On Tue, Dec 12, 2017 at 02:47:37PM +0000, Finn Christensen wrote:
> +struct flow_mark {
> + struct cmap megaflow_to_mark;
> + struct cmap mark_to_flow;
> + struct id_pool *pool;
> + struct ovs_mutex mutex;
>
> [Finn] Is this mutex needed? - the structure seems to be used by the offload
> thread only.
Nice catch, yes, it's not needed. and it's been fixed in v5 (which will be
sent out soon).
> + info.flow_mark = mark;
> + ret = netdev_flow_put(port->netdev, match,
> + CONST_CAST(struct nlattr *, actions),
> + actions_len, &flow->mega_ufid, &info, NULL);
> + if (ret) {
> + VLOG_ERR("failed to %s netdev flow with mark %u\n", op, mark);
> + flow_mark_free(mark);
>
> [Finn] If "modification" is true, then an existing flow has been deleted in
> netdev-dpdk, while calling netdev_flow_put, and the new flow could not be
> offloaded. But the old, deleted flow still has associations here. I think it
> should be disassociated here also.
>
I think you are right. I was thinking more about the "add" case, then we
should delete the reclaim the mark id if it fails. For "modification", I
will just ido disassociation, and let it to handle the mark id reclaim (
if there is no references any more for that mark id).
Thanks for your view, BTW!
--yliu
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
