Support ELL >=0.69 #302
Comments
|
Hi @jonassmedegaard, Thank you for the bug report.
Did they do that with version 0.68? Because mptcpd also needs to be adapted to support it, see #299. Maybe they don't know/remember they have to bump the API :) I guess the changes should not be too important, but personally, I don't know that code and I have deadlines approaching :-/ |
|
@matttbe I’ll take a look at the updates necessary to support the ELL API tonight. |
|
Thanks for for quick reactions! It is not currently blocking iwd, because I messed up and went ahead upgrading both libell and iwd - so it is kinda worse: If I don't get a fix for mptcpd then I'll need to roll back those releases with ugly dummy superseding versions clamped on ( 0.69.really0.68) and then be blocked... |
|
@ossama-othman thank you :) @jonassmedegaard I see. Hopefully the changes will not be too important. |
|
The change to ELL in the 0.69 release does indeed break link-time compatibility but the removed symbols still exist as AFAICT, the iwd change you referred to doesn't directly avoid those symbols since they are still used elsewhere in the iwd nl80211util.c source file. |
ossama-othman
added
question
|
@jonassmedegaard: thanks to @ossama-othman's work, mptcpd can now be compiled with ELL's development version again. Does his patch fix the issue you saw on Debian side? |
We recently had some issues because the ELL API has been broken without notice. Simply adding a cron to run the tests once a day during the week should tell us when an incompatible change has been introduced on ELL side, so we can get a bit of time to prepare a fix and a new release. Link: multipath-tcp#302 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
* gh: daily validation of the ELL compatibility We recently had some issues because the ELL API has been broken without notice. Simply adding a cron to run the tests once a day during the week should tell us when an incompatible change has been introduced on ELL side, so we can get a bit of time to prepare a fix and a new release. Link: #302 Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> * gh: switch to checkout v4 The previous version is deprecated, each job gets this warning: The following actions uses node12 which is deprecated and will be forced to run on node16: actions/checkout@v2. For more info: https://github.blog/changelog/2023-06-13-github-actions-all-actions-will-run-on-node16-instead-of-node12-by-default/ No further changes needed: we use the public runners with ubuntu-latest. Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> * gh: coveralls: switch to v2 The previous version is deprecated, each job using it gets this warning: The following actions uses a deprecated Node.js version and will be forced to run on node20: coverallsapp/github-action@master. For more info: https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/ We just have to use 'file' instead of 'path-to-lcov' which is deprecated. No further changes needed: we use the public runners with ubuntu-latest. Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --------- Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
@ossama-othman I tried to recompile mptcpd 0.12 with your patch from #303, and I got the following errors: It looks like an issue on ELL header files, no? |
When looking at the code of other projects using ELL (IWD, BlueZ,
Ofono), it looks like only 'ell.h' should be included, not individual
header files from the 'ell' header directory.
That looks like the way to go, because when looking at ell/genl.h, it
uses functions declared in ell/netlink.h, without including this file
before. This causes issues when compiling the code using libell-dev
installed on the system:
libtool: compile: gcc (...) -c path_manager.c (...)
In file included from path_manager.c:21:
/usr/include/ell/genl.h: In function 'l_genl_attr_next':
/usr/include/ell/genl.h:98:16: error: implicit declaration of function 'l_netlink_attr_next'; did you mean 'l_genl_attr_next'? [-Wimplicit-function-declaration]
98 | return l_netlink_attr_next((struct l_netlink_attr *) attr,
| ^~~~~~~~~~~~~~~~~~~
| l_genl_attr_next
/usr/include/ell/genl.h: In function 'l_genl_attr_recurse':
/usr/include/ell/genl.h:105:16: error: implicit declaration of function 'l_netlink_attr_recurse'; did you mean 'l_genl_attr_recurse'? [-Wimplicit-function-declaration]
105 | return l_netlink_attr_recurse((struct l_netlink_attr *) attr,
| ^~~~~~~~~~~~~~~~~~~~~~
| l_genl_attr_recurse
make[2]: *** [Makefile:597: libmptcpd_la-path_manager.lo] Error 1
All .c files including ELL header files have been modified to include
only <ell/ell.h>. The .cpp file in the tests has not been modified,
because it looks like that causes some issues. For the same reason,
include/mptcpd/private/plugin.h file has not been modified as well.
Closes: multipath-tcp#302
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
|
Apparently, mptcpd should only include |
ELL has dropped two symbols since version 0.69:
l_genl_attr_nextand_genl_attr_recurse.At first I just sloppily assumed they knew what they were doing and that change was harmless, since they didn't bump the API, but releasing the new version of ELL to Debian immediately made iwd fail, and a search revealed that also mptcpd directly rely on those symbols.
For inspiration, here's the change iwd made to get rid of those symbols: https://git.kernel.org/pub/scm/network/wireless/iwd.git/commit/?id=77cf621
The text was updated successfully, but these errors were encountered: