rule: Rule.Type to allow adding/listing unreachable (RTN_UNREACHABLE)… #1006
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oxtoacart
added a commit
to tailscale/netlink
that referenced
this pull request
Aug 22, 2024
This allows Tailscale to use this module while we wait for upstream PR to be merged. See vishvananda#1006 Updates vishvananda#710 Signed-off-by: Percy Wegmann <percy@tailscale.com>
oxtoacart
added a commit
to tailscale/netlink
that referenced
this pull request
Aug 22, 2024
This allows Tailscale to use this module while we wait for upstream PR to be merged. See vishvananda#1006 Updates tailscale/tailscale#12298 Signed-off-by: Percy Wegmann <percy@tailscale.com>
oxtoacart
added a commit
to tailscale/tailscale
that referenced
this pull request
Aug 22, 2024
…equire vishvananda/netlink After the upstream PR is merged, we can point directly at github.com/vishvananda/netlink and retire github.com/tailscale/netlink. See vishvananda/netlink#1006 Updates #12298 Signed-off-by: Percy Wegmann <percy@tailscale.com>
oxtoacart
force-pushed
the
percy/issue12298
branch
2 times, most recently
from
6ccfaba
to
07d06f6
Compare
oxtoacart
added a commit
to tailscale/netlink
that referenced
this pull request
Aug 22, 2024
This allows Tailscale to use this module while we wait for upstream PR to be merged. See vishvananda#1006 Updates tailscale/tailscale#12298 Signed-off-by: Percy Wegmann <percy@tailscale.com>
oxtoacart
added a commit
to tailscale/tailscale
that referenced
this pull request
Aug 22, 2024
…equire vishvananda/netlink After the upstream PR is merged, we can point directly at github.com/vishvananda/netlink and retire github.com/tailscale/netlink. See vishvananda/netlink#1006 Updates #12298 Signed-off-by: Percy Wegmann <percy@tailscale.com>
aboch
reviewed
Aug 22, 2024
rule.go
Outdated
| // Type is the unix.RTN_* rule type, such as RTN_UNICAST | ||
| // or RTN_UNREACHABLE. | ||
| // When adding a new rule, zero means automatic. | ||
| // Only supported on Linux. |
There was a problem hiding this comment.
Why does this element need all this explanation while all other ones don't?
There was a problem hiding this comment.
I'm happy to remove it if you prefer, it just seemed like useful documentation for any consumer of this new field.
There was a problem hiding this comment.
Yes let's remove it, unless it deviates from iproute2.
Assumption is who uses this library is familiar with iproute2 and can always look for reference uses there and check kernek code as well. Also you are adding UT which shows an example, which is great.
Thank you for your contribution.
oxtoacart
force-pushed
the
percy/issue12298
branch
from
07d06f6
to
e35bc30
Compare
aboch
reviewed
Aug 22, 2024
rule_linux.go
Outdated
| case unix.RTN_UNICAST: | ||
| return "" | ||
| case unix.RTN_LOCAL: | ||
| return " local" |
There was a problem hiding this comment.
I am not sure about having the leading space in the string. Let's have the print function add that. Not a big deal if we have a trailing space in the route entry when type == RTN_UNSPEC
There was a problem hiding this comment.
Done. Note that on Mac, we will always have a trailing space.
aboch
reviewed
Aug 22, 2024
rule.go
Outdated
| @@ -41,8 +42,8 @@ func (r Rule) String() string { | |||
| to = r.Dst.String() | |||
| } | |||
|
|
|||
| return fmt.Sprintf("ip rule %d: from %s to %s table %d", | |||
| r.Priority, from, to, r.Table) | |||
| return fmt.Sprintf("ip rule %d: from %s to %s table %d%s", | |||
There was a problem hiding this comment.
I am referring to this stringification of the IP Rule put a space here please
oxtoacart
force-pushed
the
percy/issue12298
branch
2 times, most recently
from
33bee70
to
5d7411d
Compare
oxtoacart
added a commit
to tailscale/tailscale
that referenced
this pull request
Aug 22, 2024
…equire vishvananda/netlink After the upstream PR is merged, we can point directly at github.com/vishvananda/netlink and retire github.com/tailscale/netlink. See vishvananda/netlink#1006 Updates #12298 Signed-off-by: Percy Wegmann <percy@tailscale.com>
…BLE) rules Updates vishvananda#710 Co-authored-by: Brad Fitzpatrick <bradfitz@tailscale.com> Signed-off-by: Percy Wegmann <percy@tailscale.com>
oxtoacart
force-pushed
the
percy/issue12298
branch
from
5d7411d
to
db6fb0a
Compare
|
LGTM |
|
Thanks for the review @aboch ! |
Asutorufa
pushed a commit
to Asutorufa/tailscale
that referenced
this pull request
Aug 23, 2024
…equire vishvananda/netlink After the upstream PR is merged, we can point directly at github.com/vishvananda/netlink and retire github.com/tailscale/netlink. See vishvananda/netlink#1006 Updates tailscale#12298 Signed-off-by: Percy Wegmann <percy@tailscale.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
… rules
Updates #710