On 23 May 2023, at 12:39, Frode Nordahl wrote:
> The tc module combines the use of the `tc_transact` helper > function for communication with the in-kernel tc infrastructure > with assertions on the reply data by `ofpbuf_at_assert` on the > received data prior to further processing. > > `tc_transact` in turn calls `nl_transact`, which via > `nl_transact_multiple__` ultimately calls and handles return > value from `recvmsg`. On error a check for EAGAIN is performed > and a consequence of this condition is effectively to provide a > non-error (0) result and an empty reply buffer. > > Before this change, the `tc_transact` and, consumers of it, were > unaware of this condition. The use of assertions on the reply > buffer can as such lead to a fatal crash of OVS. > > To be fair, the behavior of `nl_transact` when handling an EAGAIN > return is currently not documented, so this change also adds that > documentation. > > While fixing the problem, it led me to find potential problems > with the one-time initialization functions in the netdev-offload-tc > module. Make use of the information now available about an EAGAIN > condition to retry one-time initialization, and resort to logging > a warning if probing of tc features fails due to temporary > situations such as resource depletion. > > For the record, the symptom of the crash is this in the log: > EMER|../include/openvswitch/ofpbuf.h:194: assertion offset + size <= b->size > failed in ofpbuf_at_assert() > > And an excerpt of the backtrace looks like this: > 0x0000561dac1396d1 in ofpbuf_at_assert (b=0x7fb650005d20, b=0x7fb650005d20, > offset=16, size=20) at ../include/openvswitch/ofpbuf.h:194 > tc_replace_flower (id=<optimized out>, flower=<optimized out>) at > ../lib/tc.c:3223 > 0x0000561dac128155 in netdev_tc_flow_put (netdev=0x561dacf91840, > match=<optimized out>, actions=<optimized out>, actions_len=<optimized out>, > ufid=<optimized out>, info=<optimized out>, stats=<optimized out>) at > ../lib/netdev-offload-tc.c:2096 > 0x0000561dac117541 in netdev_flow_put (stats=<optimized out>, > info=0x7fb65b7ba780, ufid=<optimized out>, act_len=<optimized out>, > actions=<optimized out>, > match=0x7fb65b7ba980, netdev=0x561dacf91840) at ../lib/netdev-offload.c:257 > parse_flow_put (put=0x7fb65b7bcc50, dpif=0x561dad0ad550) at > ../lib/dpif-netlink.c:2297 > try_send_to_netdev (op=0x7fb65b7bcc48, dpif=0x561dad0ad550) at > ../lib/dpif-netlink.c:2384 > > Reported-At: https://launchpad.net/bugs/2018500 > Fixes: f98e418fbdb6 ("tc: Add tc flower functions") > Fixes: 407556ac6c90 ("netlink-socket: Avoid forcing a reply for final message > in a transaction.") > Acked-by: Roi Dayan <r...@nvidia.com> > Reviewed-by: Simon Horman <simon.hor...@corigine.com> > Signed-off-by: Frode Nordahl <frode.nord...@canonical.com> Thanks Frode for the v4, it looks good to me. And if you sent out a v5 with only Ilya’s suggested change you can keep my ack. Acked-by: Eelco Chaudron <echau...@redhat.com> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
