lwip: add support for common netif API#9343
Conversation
miri64
added
Type: enhancement
miri64
added
the
State: WIP
miri64
force-pushed
the
lwip/api/initial-netif
branch
from
9a91dc6 to
17aed2a
Compare
smlng
left a comment
smlng
left a comment
There was a problem hiding this comment.
not some change requests but rather request for explanation 😉
pkg/lwip/contrib/netif/lwip_netif.c
Outdated
| res = dev->driver->get(dev, opt, netif->hwaddr, | ||
| sizeof(netif->hwaddr)); | ||
| if (res > 0) { | ||
| netif->hwaddr_len = res; |
There was a problem hiding this comment.
could this actually be different then the one reported by the get above? I would rather think that if this get fails the hwaddr_len should be set to 0 to indicate an error, as we cannot do so due to void function.
There was a problem hiding this comment.
Nope, I guess this is copy-pasta from the GNRC version (where I did it a little differently). Will do as you proposed.
There was a problem hiding this comment.
In fact, error-checking isn't necessary here at all, since this function is only called if the address was set on the driver. If the driver doesn't allow for this, netif->hwaddr and netif->hwaddr_len shouldn't be changed anyway (which they wouldn't if get()/set() are implemented correctly by the driver ;-).
| case NETOPT_IPV6_ADDR_REMOVE: | ||
| return sizeof(ip6_addr_t); | ||
| case NETOPT_IPV6_ADDR_FLAGS: | ||
| return sizeof(uint8_t); |
There was a problem hiding this comment.
maybe its too early, but can these are no-ops, right? Can you please explain the reasoning.
There was a problem hiding this comment.
This is the WIP part of this PR ;-) the proper functions will be called here later.
| switch (opt) { | ||
| #if LWIP_IPV6 | ||
| case NETOPT_IPV6_IID: | ||
| return sizeof(eui64_t); |
There was a problem hiding this comment.
same as for the set_opt, I don't understand this - and I also think this is wrong because the docu says:
* @return Number of bytes written to @p value.
but here nothing is written to value, or did I miss something?!
There was a problem hiding this comment.
and if this assumes value is 0: I would rather not count on that, and explicitly set the default value.
There was a problem hiding this comment.
This is the WIP part of this PR ;-) the proper functions will be called here later.
Dito
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
stale
bot
added
the
State: stale
miri64
added
State: don't stale
miri64
added
the
State: archived
|
|
2 participants
Contribution description
This adds support for our common netif API for lwIP. It is still WIP. TODOs:
Issues/PRs references
Addresses #9287