Date: Fri, 7 Apr 2023 14:34:30 +0800 From: Zhenlei Huang <zlei@FreeBSD.org> To: Gleb Smirnoff <glebius@freebsd.org> Cc: hselasky@freebsd.org, kib@freebsd.org, bz@freebsd.org, gallatin@freebsd.org, current@freebsd.org Subject: Re: IFF_KNOWSEPOCH -> IFF_NEEDSEPOCH Message-ID: <2F8A1A63-642C-44C2-9319-A750B381EC70@FreeBSD.org> In-Reply-To: <ZC8Qzq3kcYLfq9Gl@FreeBSD.org> References: <ZC8Qzq3kcYLfq9Gl@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_4B2E8384-2205-4739-9F01-C10F9B8F1818 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=us-ascii > On Apr 7, 2023, at 2:34 AM, Gleb Smirnoff <glebius@freebsd.org> wrote: > > Hi, > > recently we had several drivers marked with IFF_KNOWSEPOCH > which reminded me that this flag was supposed to be temporary. > > Here is the change that introduced it e87c4940156. It was > caused by several drivers sending packets from non-interrupt > context and thus triggering NET_EPOCH_ASSERT(). It was about > 10 - 20 drivers having this problem initially and reduced down > to a few after 4426b2e64bd. We had a pretty heated dicussion > back then and I apologize for that. > > May I suggest before entering FreeBSD 14.0-RELEASE cycle we > will get back to what was there before e87c4940156? It sounds good if only a few drivers need IFF_NEEDSEPOCH. > > To avoid the driver fallout that we used to have back in > early 2020, here is the plan. In ether_input() where we > now conditionally on the IFF_KNOWSEPOCH flag enter/exit the > epoch with INVARIANTS we will also conditionally enter/exit > in case we are supposed to be in the epoch wrt the flag, but > we are not. We will also print a warning once, like "interface > foo0 called if_input without epoch". This handling will be > converted to normal assertion after a couple months. Should also apply to infiniband_input(). BTW, is `in_epoch()` too heavy to replace flag IFF_NEEDSEPOCH or IFF_KNOWSEPOCH ? > > If everybody is fine with this suggestion I will post > a review. Otherwise please express your concerns. Good to make it moving forward ;) Best regards, Zhenlei > > -- > Gleb Smirnoff --Apple-Mail=_4B2E8384-2205-4739-9F01-C10F9B8F1818 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii <html><head><meta http-equiv=3D"Content-Type" content=3D"text/html; = charset=3Dus-ascii"></head><body style=3D"word-wrap: break-word; = -webkit-nbsp-mode: space; line-break: after-white-space;" class=3D""><br = class=3D""><div><br class=3D""><blockquote type=3D"cite" class=3D""><div = class=3D"">On Apr 7, 2023, at 2:34 AM, Gleb Smirnoff <<a = href=3D"mailto:glebius@freebsd.org" class=3D"">glebius@freebsd.org</a>>= wrote:</div><br class=3D"Apple-interchange-newline"><div class=3D""><div = class=3D""> Hi,<br class=3D""><br class=3D"">recently we had = several drivers marked with IFF_KNOWSEPOCH<br class=3D"">which reminded = me that this flag was supposed to be temporary.<br class=3D""><br = class=3D"">Here is the change that introduced it e87c4940156. It was<br = class=3D"">caused by several drivers sending packets from = non-interrupt<br class=3D"">context and thus triggering = NET_EPOCH_ASSERT(). It was about<br class=3D"">10 - 20 drivers having = this problem initially and reduced down<br class=3D"">to a few after = 4426b2e64bd. We had a pretty heated dicussion<br class=3D"">back then = and I apologize for that.<br class=3D""><br class=3D"">May I suggest = before entering FreeBSD 14.0-RELEASE cycle we<br class=3D"">will get = back to what was there before e87c4940156?<br = class=3D""></div></div></blockquote><div><br class=3D""></div><div>It = sounds good if only a few drivers need IFF_NEEDSEPOCH.</div><br = class=3D""><blockquote type=3D"cite" class=3D""><div class=3D""><div = class=3D""><br class=3D"">To avoid the driver fallout that we used to = have back in<br class=3D"">early 2020, here is the plan. In = ether_input() where we<br class=3D"">now conditionally on the = IFF_KNOWSEPOCH flag enter/exit the<br class=3D"">epoch with INVARIANTS = we will also conditionally enter/exit<br class=3D"">in case we are = supposed to be in the epoch wrt the flag, but<br class=3D"">we are not. = We will also print a warning once, like "interface<br class=3D"">foo0 = called if_input without epoch". This handling will be<br = class=3D"">converted to normal assertion after a couple months.<br = class=3D""></div></div></blockquote><div><br class=3D""></div>Should = also apply to infiniband_input().<br class=3D""><div><br = class=3D""></div><div>BTW, is `in_epoch()` too heavy to replace = flag <span style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, = 0);" class=3D"">IFF_NEEDSEPOCH</span></div><div><span = style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" = class=3D"">or </span><font color=3D"#000000" class=3D""><span = style=3D"caret-color: rgb(0, 0, 0);" class=3D"">IFF_KNOWSEPOCH = ?</span></font></div><br class=3D""><blockquote type=3D"cite" = class=3D""><div class=3D""><div class=3D""><br class=3D"">If everybody = is fine with this suggestion I will post<br class=3D"">a review. = Otherwise please express your concerns.<br = class=3D""></div></div></blockquote><div><br class=3D""></div><div>Good = to make it moving forward ;)</div><div><br class=3D""></div><div><div = style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Best = regards,</div><div style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, = 0);">Zhenlei</div></div><br class=3D""><blockquote type=3D"cite" = class=3D""><div class=3D""><div class=3D""><br class=3D"">-- <br = class=3D"">Gleb Smirnoff<br class=3D""></div></div></blockquote></div><div= class=3D""><div><br class=3D""></div> </div> <br class=3D""></body></html>= --Apple-Mail=_4B2E8384-2205-4739-9F01-C10F9B8F1818--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2F8A1A63-642C-44C2-9319-A750B381EC70>