lwip: add support for common netif API #9343
Conversation
9a91dc6 to
17aed2a
Compare
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. |
|
|
Contribution description
This adds support for our common netif API for lwIP. It is still WIP. TODOs:
Issues/PRs references
Addresses #9287