At the very least, it CAN happen that no OVS key attribute is found in the keyAttr array. I don't know exactly what causes this as it is difficult to reproduce as it always ends in a BSOD on the host. But maybe Alin can explain it better.
I only found this by going through the kernel debug log. What I have seen in this case is that the packet is just not sent but without OVS attributes it never had a chance to be sent. And yes, all ASSERTs in the kernel datapath are useless in the release build because they are not included in the release build. I'm not a kernel dev expert, but I would say that relying only on an assert instead of explicitly checking for NULL is always a risk. So, removing the assert would make sense I would say. ________________________________ Von: Mike Pattrick <[email protected]> Gesendet: Dienstag, 4. März 2025 15:31 An: Frank Wagner <[email protected]> Cc: [email protected] <[email protected]> Betreff: Re: [ovs-dev] [PATCH] windows: Fixed BSOD in kernel driver. On Sat, Mar 1, 2025 at 7:35 AM Frank Wagner <[email protected]> wrote: > > It can happen that ovs key attributes are not in keyAttrs of port. > In this case the call of NlAttrGetU32 will cause a BSOD in Release builds. > > Signed-off-by: Frank Wagner <[email protected]> > > --- > datapath-windows/ovsext/User.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c > index c4563b28b..b52124abf 100644 > --- a/datapath-windows/ovsext/User.c > +++ b/datapath-windows/ovsext/User.c > @@ -407,7 +407,9 @@ _MapNlAttrToOvsPktExec(PNL_MSG_HDR nlMsgHdr, PNL_ATTR > *nlAttrs, > execute->actionsLen = NlAttrGetSize(nlAttrs[OVS_PACKET_ATTR_ACTIONS]); > > ASSERT(keyAttrs[OVS_KEY_ATTR_IN_PORT]); > - execute->inPort = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_IN_PORT]); > + if (keyAttrs[OVS_KEY_ATTR_IN_PORT]) { > + execute->inPort = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_IN_PORT]); > + } Hello Frank, Is this expected behaviour? If so then the assert above should probably be removed. What is the InPort expected to be set to when this attribute is missing? What would cause InPort to be NULL in this case? Cheers, M > execute->keyAttrs = keyAttrs; > > if (nlAttrs[OVS_PACKET_ATTR_MRU]) { > -- > 2.48.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
