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