On Mon, Oct 16, 2023 at 4:59 PM Jamal Hadi Salim [off-list ref] wrote:

On Mon, Oct 16, 2023 at 4:38 PM Jamal Hadi Salim [off-list ref] wrote:

quoted

On Mon, Oct 16, 2023 at 4:15 PM Jakub Kicinski [off-list ref] wrote:

quoted

On Mon, 16 Oct 2023 05:35:31 -0400 Jamal Hadi Salim wrote:

quoted

Changes In RFC Version 7
-------------------------

0) First time removing the RFC tag!

1) Removed XDP cookie. It turns out as was pointed out by Toke(Thanks!) - that
using bpf links was sufficient to protect us from someone replacing or deleting
a eBPF program after it has been bound to a netdev.

2) Add some reviewed-bys from Vlad.

3) Small bug fixes from v6 based on testing for ebpf.

4) Added the counter extern as a sample extern. Illustrating this example because
   it is slightly complex since it is possible to invoke it directly from
   the P4TC domain (in case of direct counters) or from eBPF (indirect counters).
   It is not exactly the most efficient implementation (a reasonable counter impl
   should be per-cpu).

I think that I already shared my reservations about this series.

And please please let's have a _technical_ discussion on reservations
not hyperboles.

quoted

On top of that, please, please, please make sure that it builds cleanly
before posting.

I took the shared infra 8 hours to munch thru this series, and it threw
out all sorts of warnings. 8 hours during which I could not handle any
PR or high-prio patch :( Not your fault that builds are slow, I guess,
but if you are throwing a huge series at the list for the what-ever'th
time, it'd be great if it at least built cleanly :(

We absolutely dont want to add unnecessary work.
Probably we may have missed the net-next tip? We'll pull the latest
and retest with tip.
Is there a link that we can look at on what the infra does so we can
make sure it works per expectation next time?
If you know what kind of warnings/issues so we can avoid it going forward?
Note: We didnt see any and we built each patch separately on gcc 11,
12, 13 and clang 16.
BTW: Lore does reorder the patches, but i am assuming cicd is smart
enough to understand this?

Verified from downloading mbox.gz from lore that the tarball was
reordered. Dont know if it contributed - but now compiling patch by
patch on the latest net-next tip.

Never mind - someone pointed me to patch work and i can see some
warnings there. Looks like we need more build types and compiler
options to catch some of these issues.
We'll look into it and we will replicate in our cicd.

cheers,
jamal

cheers,
jamal

quoted

quoted

--
pw-bot: cr