My 2cents on this one, this looks like a symptom if its not always
reproducible. There could be another underlying issue that is corrupting
the memory and triggering the aforementioned symptom.
Can you please add more details on how this can be reproduced (which type
of traffic, topology etc)?
On Tue, Mar 4, 2025 at 10:24 PM Frank Wagner <frank.wag...@dbosoft.eu>
wrote:
> 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 <m...@redhat.com>
> *Gesendet:* Dienstag, 4. März 2025 15:31
> *An:* Frank Wagner <frank.wag...@dbosoft.eu>
> *Cc:* d...@openvswitch.org <d...@openvswitch.org>
> *Betreff:* Re: [ovs-dev] [PATCH] windows: Fixed BSOD in kernel driver.
>
> On Sat, Mar 1, 2025 at 7:35 AM Frank Wagner <frank.wag...@dbosoft.eu>
> 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 <frank.wag...@dbosoft.eu>
> >
> > ---
> > 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
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
