-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
zebra: Extend the FPM module to push the missing SRv6 information #12301
zebra: Extend the FPM module to push the missing SRv6 information #12301
Conversation
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 3: Failed (click for details)Topotests Ubuntu 18.04 amd64 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-8301/artifact/TP3U1804AMD64/ErrorLog/ Topotests Ubuntu 18.04 amd64 part 3: No useful log foundSuccessful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsTopotests Ubuntu 18.04 amd64 part 3: Failed (click for details)Topotests Ubuntu 18.04 amd64 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-8301/artifact/TP3U1804AMD64/ErrorLog/ Topotests Ubuntu 18.04 amd64 part 3: No useful log found
|
ci:rerun |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-8302/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
|
@cscarpitta which one should we start reviewing, this or #12292? :) |
@ton31337 This, thanks. I will close the other one to avoid confusion :) |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
ac14b05
to
84d8e94
Compare
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-8359/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
|
Rebased to resolve merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had some initial questions
FPM_NH_ENCAP_SRV6_LOCAL_SID = 102, | ||
}; | ||
|
||
enum { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not crazy about these anonymous enums - do we have guidance about that in the dev docs?
}; | ||
|
||
enum { | ||
SRV6_ROUTE_UNSPEC = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why put these SRV6 items here - why not in one of the srv6-specific headers?
}; | ||
|
||
enum srv6_localsid_action { | ||
FPM_SRV6_LOCALSID_ACTION_UNSPEC = 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FPM-specific items - is this the right header for FPM?
@@ -406,6 +406,29 @@ vrf_id_t vrf_lookup_by_table(uint32_t table_id, ns_id_t ns_id) | |||
return VRF_DEFAULT; | |||
} | |||
|
|||
static struct zebra_vrf *vrf_lookup_by_table_id(uint32_t table_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't really care for the copy-paste of the existing function logic. can't we have one version of the logic that knows the rules for iterating through the vrfs, and then use that to return the different pieces of info? and really, that should be in the zebra_vrf module, shouldn't it?
ctx = &nexthop->nh_srv6->seg6local_ctx; | ||
|
||
if (!nl_attr_put16(nlmsg, req_size, RTA_ENCAP_TYPE, | ||
FPM_NH_ENCAP_SRV6_LOCAL_SID)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so the design here is "pick a new encap number, and use that to contain the various tlvs with srv6 info", is that right?
is that a robust choice - will the OS-based netlink messaging never support this information? the original goal of the netlink encoded FPM messaging was to be able to use a single encoding or marshalling code-base consistently, but this sort of pushes that aside and introduces a different approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mjstapp Thanks for your review.
Yes, we pick a new encap number and we use that to contain the TLVs with SRv6 information.
I know dplane_fpm_nl
is designed to share zebra's Netlink encoding functions but unfortunately we can't reuse the existing encoding functions in this case.
The reason is that the SRv6 kernel's data model is very different from the SRv6 SONiC's data model.
Let's make an example.
When we encapsulate a packet using SRv6, we add an outer IPv6 header and we need to choose an IPv6 address to use as source address in the outer IPv6 header.
This is implemented differently in the Linux kernel and in SONiC.
In the Linux kernel, we have a source address configured globally in the system and we use that address for all the SRv6 encapsulated packets. For this reason, the Netlink message used to install a route in the kernel does not contain the source address.
Instead, in SONiC each SRv6 route has its own IPv6 source address. Therefore, the Netlink message used to install an SRv6 route in SONiC must contain the source address.
This is why we need to pick a new encap number to send the SRv6 information to SONiC.
This PR is stale because it has been open 180 days with no activity. Comment or remove the |
This commit adds the `encap_src_addr` parameter to the `struct zebra_srv6`. This parameter represents the source address of the outer encapsulating IPv6 header. Support for configuring the `encap_src_addr`parameter will be added in a future commit. Signed-off-by: Carmine Scarpitta <[email protected]>
This commit prints an error message when `netlink_route_nexthop_encap()` receives a nexthop with an unknown encap type. Signed-off-by: Carmine Scarpitta <[email protected]>
This commit adds a new parameter `fpm` to the `_netlink_route_build_singlepath()` function. This parameter lets `_netlink_route_build_singlepath()` know if a route is installed via FPM or not. This is useful for implementing use cases where a route must be encoded differently depending on whether it is installed via FPM or not. Signed-off-by: Carmine Scarpitta <[email protected]>
Zebra’s `dplane_fpm_nl` module allows an external component to learn the forwarding information computed by the FRR routing suite. Currently, the SRv6 routes exported by the `dplane_fpm_nl` module do not include some information such as the source address for the SRv6 encapsulation. This commit adds the missing information to the SRv6 routes exported by the `dplane_fpm_nl`. Signed-off-by: Carmine Scarpitta <[email protected]>
Zebra’s `dplane_fpm_nl` module allows an external component to learn the forwarding information computed by the FRR routing suite. Currently, the SRv6 Local SIDs exported by the `dplane_fpm_nl` module do not include some information such the SID format (i.e., locator block length, locator node length, function length, and argument length). This commit adds the missing information to the SRv6 Local SIDs exported by the `dplane_fpm_nl`. Signed-off-by: Carmine Scarpitta <[email protected]>
84d8e94
to
b1dea24
Compare
Rebased on master branch. |
Continuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedUbuntu 18.04 amd64 build: Failed (click for details)Ubuntu 18.04 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U1804AMD64/config.log/config.log.gz Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U1804AMD64/config.status/config.statusMake failed for Ubuntu 18.04 amd64 build:
Ubuntu 18.04 i386 build: Failed (click for details)Make failed for Ubuntu 18.04 i386 build:
Ubuntu 18.04 i386 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U18I386BUILD/config.status/config.status Ubuntu 18.04 arm7 build: Failed (click for details)Ubuntu 18.04 arm7 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U18ARM7BUILD/config.status/config.status Ubuntu 18.04 arm7 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U18ARM7BUILD/config.log/config.log.gzMake failed for Ubuntu 18.04 arm7 build:
Debian 11 amd64 build: Failed (click for details)Debian 11 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/DEB11AMD64/config.log/config.log.gzMake failed for Debian 11 amd64 build:
Debian 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/DEB11AMD64/config.status/config.status Redhat 8 amd64 build: Failed (click for details)Redhat 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/REDHAT8/config.status/config.statusMake failed for Redhat 8 amd64 build:
Redhat 8 amd64 build: Unknown Log <config.log.gz> Ubuntu 18.04 arm8 build: Failed (click for details)Make failed for Ubuntu 18.04 arm8 build:
Ubuntu 18.04 arm8 build: Unknown Log <config.log.gz> Debian 10 amd64 build: Failed (click for details)Debian 10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/DEB10BUILD/config.status/config.statusMake failed for Debian 10 amd64 build:
Debian 10 amd64 build: Unknown Log <config.log.gz> Debian 9 amd64 build: Failed (click for details)Debian 9 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/CI021BUILD/config.log/config.log.gz Debian 9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/CI021BUILD/config.status/config.statusMake failed for Debian 9 amd64 build:
Ubuntu 20.04 amd64 build: Failed (click for details)Ubuntu 20.04 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U2004AMD64BUILD/config.log/config.log.gzMake failed for Ubuntu 20.04 amd64 build:
Ubuntu 20.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U2004AMD64BUILD/config.status/config.status Ubuntu 18.04 ppc64le build: Failed (click for details)Ubuntu 18.04 ppc64le build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U1804PPC64LEBUILD/config.log/config.log.gz Ubuntu 18.04 ppc64le build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U1804PPC64LEBUILD/config.status/config.statusMake failed for Ubuntu 18.04 ppc64le build:
Redhat 9 amd64 build: Failed (click for details)Redhat 9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/RH9BUILD/config.status/config.statusMake failed for Redhat 9 amd64 build:
Redhat 9 amd64 build: Unknown Log <config.log.gz> Ubuntu 22.04 amd64 build: Failed (click for details)Ubuntu 22.04 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U22AMD64BUILD/config.log/config.log.gzMake failed for Ubuntu 22.04 amd64 build:
Ubuntu 22.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U22AMD64BUILD/config.status/config.status Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsUbuntu 18.04 amd64 build: Failed (click for details)Ubuntu 18.04 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U1804AMD64/config.log/config.log.gz Ubuntu 18.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U1804AMD64/config.status/config.statusMake failed for Ubuntu 18.04 amd64 build:
Ubuntu 18.04 i386 build: Failed (click for details)Make failed for Ubuntu 18.04 i386 build:
Ubuntu 18.04 i386 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U18I386BUILD/config.status/config.status Ubuntu 18.04 arm7 build: Failed (click for details)Ubuntu 18.04 arm7 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U18ARM7BUILD/config.status/config.status Ubuntu 18.04 arm7 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U18ARM7BUILD/config.log/config.log.gzMake failed for Ubuntu 18.04 arm7 build:
Debian 11 amd64 build: Failed (click for details)Debian 11 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/DEB11AMD64/config.log/config.log.gzMake failed for Debian 11 amd64 build:
Debian 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/DEB11AMD64/config.status/config.status Redhat 8 amd64 build: Failed (click for details)Redhat 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/REDHAT8/config.status/config.statusMake failed for Redhat 8 amd64 build:
Redhat 8 amd64 build: Unknown Log <config.log.gz> Ubuntu 18.04 arm8 build: Failed (click for details)Make failed for Ubuntu 18.04 arm8 build:
Ubuntu 18.04 arm8 build: Unknown Log <config.log.gz> Debian 10 amd64 build: Failed (click for details)Debian 10 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/DEB10BUILD/config.status/config.statusMake failed for Debian 10 amd64 build:
Debian 10 amd64 build: Unknown Log <config.log.gz> Debian 9 amd64 build: Failed (click for details)Debian 9 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/CI021BUILD/config.log/config.log.gz Debian 9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/CI021BUILD/config.status/config.statusMake failed for Debian 9 amd64 build:
Ubuntu 20.04 amd64 build: Failed (click for details)Ubuntu 20.04 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U2004AMD64BUILD/config.log/config.log.gzMake failed for Ubuntu 20.04 amd64 build:
Ubuntu 20.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U2004AMD64BUILD/config.status/config.status Ubuntu 18.04 ppc64le build: Failed (click for details)Ubuntu 18.04 ppc64le build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U1804PPC64LEBUILD/config.log/config.log.gz Ubuntu 18.04 ppc64le build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U1804PPC64LEBUILD/config.status/config.statusMake failed for Ubuntu 18.04 ppc64le build:
Redhat 9 amd64 build: Failed (click for details)Redhat 9 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/RH9BUILD/config.status/config.statusMake failed for Redhat 9 amd64 build:
Redhat 9 amd64 build: Unknown Log <config.log.gz> Ubuntu 22.04 amd64 build: Failed (click for details)Ubuntu 22.04 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U22AMD64BUILD/config.log/config.log.gzMake failed for Ubuntu 22.04 amd64 build:
Ubuntu 22.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-12115/artifact/U22AMD64BUILD/config.status/config.status
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
SRv6 is supported in SONiC (sonic-net/sonic-swss#1964) as part of release 202211 (~Nov 2021). Zebra’s
dplane_fpm_nl
module allows SONiC to learn the forwarding information computed by the FRR routing suite. Currently, the information exported bydplane_fpm_nl
does not include some SRv6 fields required by SONiC.We already have two open PRs to support two functionalities required for the integration of FRR and SONiC: #12219 adds the support for SRv6 uSID behaviors to FRR; #12261 adds the missing source address parameter for the SRv6 encapsulation.
This PR completes the integration work of FRR and SONiC by extending the
dplane_fpm_nl
module to push the missing SRv6 information to SONiC.Signed-off-by: Carmine Scarpitta [email protected]