| Message ID | 1614135106-12201-1-git-send-email-maoyingming@baidu.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] dpctl/add-flow: Fix ovs-dpdk crash when add dp flow without in_port field. | expand |
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@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@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 { >
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__); + error = EINVAL; + return error; + } + if (put->ufid) { ufid = *put->ufid; } else {
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 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@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@baidu.com> --- lib/dpif-netdev.c | 6 ++++++ 1 file changed, 6 insertions(+)