[ovs-dev] [PATCH] dpctl/add-flow: Fix ovs-dpdk crash when add dp flow without in_port field.
Ilya Maximets
i.maximets at ovn.org
Wed Feb 24 12:43:11 UTC 2021
On 2/24/21 3:51 AM, Mao YingMing wrote:
> Fix the following ovs-dpdk crash:
>
> $ ovs-appctl dpctl/add-flow "eth(),eth_type(0x0800),ipv4()" "3"
> unixctl|WARN|error communicating with unix:/usr/local/var/run/openvswitch/ovs-vswitchd.1995.ctl: End of file
> ovs-appctl: ovs-vswitchd: transaction error (End of file)
>
> ovs-vswitchd.log record:
> util(ovs-vswitchd)|EMER|lib/dpif-netdev.c:3638: assertion match->wc.masks.in_port.odp_port == ODPP_NONE failed in dp_netdev_flow_add()
> daemon_unix(monitor)|ERR|2 crashes: pid 1995 died, killed (Aborted), core dumped, restarting
Good catch. Even though 'dpctl/add-flow' is a debug interface
and this should not happen in normal case, we need to fix this.
Could you, please, add a unit test for this issue to tests/dpif-netdev.at ?
Some comments inline.
Best regards, Ilya Maximets.
>
> Fix result:
> $ ovs-appctl dpctl/add-flow "eth(),eth_type(0x0800),ipv4()" "3"
> ovs-vswitchd: updating flow table (Invalid argument)
> ovs-appctl: ovs-vswitchd: server returned an error
>
> ovs-vswitchd.log record:
> dpif_netdev|ERR|missing in_port info in dpif_netdev_flow_put
> dpif|WARN|netdev at ovs-netdev: failed to put[create] (Invalid argument) ufid:c6994eb3-9c27-467d-9c41-8d235e1511b5 eth(src=00:00:00:00:00:00/00:00:00:00:00:00,dst=00:00:00:00:00:00/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=0.0.0.0/0.0.0.0,dst=0.0.0.0/0.0.0.0,proto=0/0,tos=0/0,ttl=0/0), actions:3
>
> Signed-off-by: Mao YingMing <maoyingming at baidu.com>
> ---
> lib/dpif-netdev.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index e3fd0a0..567c3d1 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -3834,6 +3834,12 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put)
> return error;
> }
>
> + if (match.wc.masks.in_port.odp_port != ODPP_NONE) {
> + VLOG_ERR("missing in_port info in %s", __func__);
We likely need the error message rate limited to avoid flood in logs
in case we ever hit that in a normal scenario. You could create a
separate rate_limit variable and use VLOG_ERR_RL macro.
The message itself might be a bit more user-friendly. Maybe something like:
VLOG_ERR_RL(&rl, "failed to put%s flow: in_port is not an exact match",
(put->flags & DPIF_FP_CREATE) ? "[create]"
: (put->flags & DPIF_FP_MODIFY) ? "[modify]" : "[zero]");
> + error = EINVAL;
No need to set error. Just return EINVAL.
> + return error;
> + }> +
> if (put->ufid) {
> ufid = *put->ufid;
> } else {
>
More information about the dev
mailing list