On Sat, Mar 8, 2025 at 6:07 AM Alin Serdean <[email protected]> wrote: > > AFAIR the assumption was that attribute can't be empty since it always has to > be written by the userspace and validated prior.
In odp_flow_key_from_flow__() the value may not be written. And making this an optional parameter matches the linux kernel module at metadata_from_nlattrs(). Cheers, M > The assert was there under debug to see if the userspace does not set it > during the initial porting. > > That being said there is nothing wrong in adding the check, it just irks me > that this might hide a bigger issue. > > Will try to reproduce it today/tomorrow and try to understand the issue > better. > > On Thu, Mar 6, 2025 at 7:51 PM Mike Pattrick <[email protected]> wrote: >> >> 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]); >> > + } >> >> After reading through the code a bit more, I believe this is nearly >> correct, but the ASSERT should be removed. >> >> However, I don't have a Windows kernel dev environment set up, so I >> can't test this out. >> >> 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
