Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebase PXE additions on smoltcp/master #1

Open
wants to merge 814 commits into
base: pxe-boot
Choose a base branch
from

Conversation

sephamorr
Copy link

Merging smoltcp/master

Dirbaio and others added 29 commits September 21, 2022 23:23
671: fix socket feature check r=Dirbaio a=M1cha

it requires you to have at least one socket type enabled but the feature
`socket-dhcp` does not exist because it's name is `socket-dhcpv4`. So if
that's only socket type you want enabled you weren't able to do that due
to this broken check.

Co-authored-by: Michael Zimmermann <[email protected]>
Co-authored-by: Dario Nieuwenhuis <[email protected]>
Previously, error handling was performed in the closure and after the
closure as well. Now, error handling is performed in one place.
667: Change egress error handling r=Dirbaio a=thvdveld

Previously, error handling was performed in the closure and after the
closure as well. Now, error handling is performed in one place.

Co-authored-by: Thibaut Vandervelden <[email protected]>
RFC 6762 Section 5.1 specifies a one-shot multicast DNS query. This
query has minimal differences from a standard DNS query, mostly just
using a multicast address and a different port (5353 vs 53).

A fully standards compliant mDNS implementation would use UDP source
port 5353 as well to issue queries, however we MUST NOT use that port
and continue using an ephemeral port until features such as service
discovery are implemented.

This change also allows specifying what kind of DNS query we wish to
perform.

https://www.rfc-editor.org/rfc/rfc6762#section-5.1
Per RFC 2132 section 9.11 the server can manually specify a renewal (T1)
time different from the default of half the lease time through option
code 58. This PR updates the behavior of the dhcp client to use that
value, if provided, and only if not provided does it default to half of
the lease duration.

Since the current state of smoltcp does not seem to follow the REBINDING
state, I also made it look for a value in option code 59 (which should
be the rebinding (T2) interval) and use that if no T1 interval interval
is provided. This behavior seems sensible to me, given that we're not
following the REBINDING part of the spec, but I can change it to ignore
option code 59, or any other handling, if that is preferred.
683: Use renewal time from DHCP server ACK, if given r=Dirbaio a=JarredAllen

# Description

Per RFC 2132 section 9.11 the server can manually specify a renewal (T1) time different from the default value (half the lease time) through option code 58. This PR updates the behavior of the dhcp client to use that value, if provided, and only if not provided does it default to half of the lease duration.

Since smoltcp seems to ignore the REBINDING state, I also made it look for a provided rebinding (T2) time provided by the server (dhcp option 59) and made it use that value as the renewal time if no renewal time was provided and the rebinding time is less than the default. This behavior seems sensible to me, given that we're not following the REBINDING part of the spec, but I can change it to ignore option code 59, or any other handling, if that is preferred.

# Verification

I realized that this functionality was missing when I changed my configuration to set a 10 second renew time on a lease which lasts for a very long time, and observed that my devices (which use this library) weren't attempting to renew. To verify that this PR works, I ran it in my existing setup and confirmed that my devices now renew their leases at approximately 10 second intervals. I think this, alongside the tests in CI, should be good enough.

Co-authored-by: Jarred Allen <[email protected]>
684: Fix 6LoWPAN fragmentation r=thvdveld a=thvdveld



Co-authored-by: Thibaut Vandervelden <[email protected]>
Swaps the bool in the raw query API to an enum that can statically
prevent mdns usage without the feature flag enabled.
669: Adds one-shot mDNS resolution r=Dirbaio a=benbrittain

RFC 6762 Section 5.1 specifies a one-shot multicast DNS query. This
query has minimal differences from a standard DNS query, mostly just
using a multicast address and a different port (5353 vs 53).

A fully standards compliant mDNS implementation would use UDP source
port 5353 as well to issue queries, however we MUST NOT use that port
and continue using an ephemeral port until features such as service
discovery are implemented.

This change also allows specifying what kind of DNS query we wish to
perform.

https://www.rfc-editor.org/rfc/rfc6762#section-5.1

Co-authored-by: Benjamin Brittain <[email protected]>
Co-authored-by: Thibaut Vandervelden <[email protected]>
\# Description

If the user provides a buffer in which to store the packets, then the
contents of the received packet will be buffered and included in the
returned Config when the DHCP connection is made. However, it isn't
included in the Config struct until the value is returned to the user,
so the equality check for whether to call `config_changed` disregards
any additional information in the buffer beyond what this library
parses.

For my purposes, I need access to the contents of the buffer when they
change as a result of a new packet, even if everything else is the same.
Since two packets will almost certainly not be the same thanks to the
magic cookie (unless the packet gets duplicated on the network, which is
an acceptably low risk for my use of smoltcp), an acceptable option for
my uses is to just always return the new configuration when a packet is
received (gated on whether a buffer is provided to return the packet
into).

\# Alternatives

While this approach is the easiest and best for my uses, I can think of
the following alternatives which would also work and might be prefered
for other use-cases:
 * Allow the user to specify whether they wish to receive all packets
   instead of opting all users who provide a buffer into this behavior
 * Allow the user to provide a closure which compares the old and new
   packets and returns true if this represents a new config which should
   be returned.
 * Compare the old packet to the new packet (either byte-by-byte or
   looking at the provided options) and only return a new config if
   differences are found.
\# Verification

In my setup, I was seeing bugs that were caused by smoltcp not exposing
the config when the only changes were in the additional options that I
want to use but which smoltcp doesn't use directly. Using this branch
instead of the main release fixed those bugs and I was able to verify
that it behaves the way I expected. I think this verification, along
with CI tests passing, should be sufficient for verifying this PR.
686: Changes egress functions to pass up Err(Exhausted) r=thvdveld a=benbrittain

Currently the poll functions will return `Ok(true)` instead of `Err(Exhausted)` despite logging about the failed transmission

Co-authored-by: Benjamin Brittain <[email protected]>
Co-authored-by: Thibaut Vandervelden <[email protected]>
687: Add address context information for resolving 6LoWPAN addresses r=thvdveld a=thvdveld

Implements [3.1.2. Context Identifier Extension](https://www.rfc-editor.org/rfc/rfc6282#section-3.1.2)

Co-authored-by: Thibaut Vandervelden <[email protected]>
681: Refactor 6LoWPAN function r=thvdveld a=thvdveld

Now, handling UDP packets is done using `process_udp`. Also removed `cfg_if!` in `interface` since that messed with formatting of the code.

It also contains a fix for the fragmentation.

Co-authored-by: Thibaut Vandervelden <[email protected]>
690: Increase version number r=thvdveld a=thvdveld

This makes testing local changes with patches work in other crates.

Co-authored-by: Thibaut Vandervelden <[email protected]>
thvdveld and others added 29 commits June 5, 2023 11:04
The payload length of the first IPv4 fragment packet contained the
length of the unfragmented packet. This was because the `repr` was
a clone and not a mutable ref to `ip_repr`. This is now fixed.

We also didn't check that the full IP packet fits in the fragmentation
buffer, which should contain the unfragmented emitted packet.

Fixes #791.

Signed-off-by: Thibaut Vandervelden <[email protected]>
792: fix(791): wrong payload length of first IPv4 frag r=thvdveld a=thvdveld

The payload length of the first IPv4 fragment packet contained the length of the unfragmented packet. This was because the `repr` was a clone and not a mutable ref to `ip_repr`. This is now fixed.

We also didn't check that the full IP packet fits in the fragmentation buffer, which should contain the unfragmented emitted packet.

Fixes #791.

Co-authored-by: Thibaut Vandervelden <[email protected]>
Switch from bors to github merge queue.
RPL: add objective function and parent set
Signed-off-by: Thibaut Vandervelden <[email protected]>
iface: add support for sending to subnet-local broadcast addrs (like 192.168.1.255).
Pass the now time when creating the iface
Signed-off-by: Thibaut Vandervelden <[email protected]>
iface: fix outdated docs on `Interface::new`
Device-level packet metadata identifiers
Pass PacketMeta separately, not within IpPacket.
@Dirbaio Dirbaio deleted the master branch June 26, 2023 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.