David Marchand, Oct 17, 2024 at 15:52:
Hello Robin,
[snip]
- Now that many changes hit the main repo, this series needs some rebasing (conflicts are not too difficult, but having this rebase run through a bit in the CI would be great).
Will do.
- From a compatibility pov, I am not fond of the rte_ip.h => rte_ip.h, rte_ip6.h split. Applications will need to be updated for something that was ambiguous so far: rte_ip.h defined all symbols, regardless of IPv4 and IPv6. What do you think of keeping a rte_ip.h compat header that just includes new rte_ip4.h and rte_ip6.h headers?
I can include rte_ip6.h in rte_ip.h for backward compatibility.
- With current patch, since only rte_ip.h is parsed in doxygen, we lost documentation for IPv6. Regardless of keeping a compat header, doc/api/doxy-api-index.md needs some update.
Got it. I will add a new "IPv6" section in "layers" and a checksum section in "basic" (rte_cksum.h split also removed some symbols from the "IP" section).
- bonus 1: It would be worth cleaning unneeded includes in rte_ip*.h. But changing this is risky if we want to take this series in rc1. Please don't do this in next revision, this will wait after merging this series. For example, running iwyu returns: lib/net/rte_ip.h should remove these lines: - #include <arpa/inet.h> // lines 26-26 - #include <netinet/ip6.h> // lines 28-28 - #include <rte_mbuf.h> // lines 33-33 - #include <sys/socket.h> // lines 23-23 - #include <sys/types.h> // lines 24-24 lib/net/rte_ip6.h should remove these lines: - #include <arpa/inet.h> // lines 26-26 - #include <netinet/ip6.h> // lines 27-27 - #include <rte_mbuf.h> // lines 31-31 - #include <sys/socket.h> // lines 23-23
Ok, I will hold off changing this for now. Btw, rte_mbuf *is* actually used in rte_ip6.h. Maybe iwyu has some troubles identifying things :)
- bonus 2: Would it be possible to provide a cocci script or some shellscript for an application to convert to the updated APIs (especially the impact on rte_flow and other structures)? We did something similar for the prefixing of dpdk structures with RTE_. Idem, not necessary for rc1.
I never used coccinelle and it may be complicated to come up with something reliable. It is not a simple renaming of symbols. It is a change of signature for a lot of functions. This would require extensive changes in the application code.
Cheers.
