openconfig / featureprofiles

Feature Profiles are groups of OpenConfig paths and tests which verify their behavior
Apache License 2.0
48 stars 138 forks source link

AddASPathSegment(asSeg...) is appending only first AS number #1659

Closed cprabha closed 1 year ago

cprabha commented 1 year ago

Hi,

With current code AddASPathSegment(asSeg...) option in ATE is appending only the first AS number.

For eg., with below code , only first AS in list 65505 is seen in the routes received in the DUT from ATE. asSeg := []uint32{65505, 65501, 65503} bgpNeti1.BGP().WithNextHopAddress(ateSrc.IPv4).AddASPathSegment(asSeg...).WithTypeSEQ()

Also gnmi get on ATE fails if "WithTypeSet" is used .

script code: ASseg := []uint32{65501, 600}, WithTypeSet option

Error: bgp_remove_private_as_test.go:263: Await(t) on ate(10.92.196.126) at /network-instances/network-instance[name=atedst]/protocols/protocol[identifier=BGP][name=0]/bgp/rib/afi-safis/afi-safi[afi-safi-name=IPV4_UNICAST]/ipv4-unicast/neighbors/neighbor[neighbor-address=]/adj-rib-in-pre/routes/route[path-id=0][prefix=]/state/prefix: error receiving gNMI response: failed to append details for elem 0: invalid AS path string: "<500 100> {65501}"

Can you please help fix this issue?

Thanks, Prabha

greg-dennis commented 1 year ago

@cprabha the error in the gnmi response when using WithTypeSet is a bug that I will fix.

The issue of "only first AS in list 65505 is seen in the routes received in the DUT from ATE" is not necessarily a bug in Ondatra. The question there is whether IxNetwork is configured correctly. Can you check whether IxNetwork is configured to have all three "65505, 65501, 65503" in the AS path segment? If it is, then the Ondatra contract (to configure IxNetwork correctly) has been met.

cprabha commented 1 year ago

Hi Greg,

I did check routes on ATE , it shows first AS number even though three as segments are set in API.

Thanks, Prabha

From: Greg Dennis @.> Sent: Thursday, May 25, 2023 3:41 PM To: openconfig/featureprofiles @.> Cc: Prabha Chethan @.>; Mention @.> Subject: Re: [openconfig/featureprofiles] AddASPathSegment(asSeg...) is appending only first AS number (Issue #1659)

[External Email. Be cautious of content]

@cprabhahttps://urldefense.com/v3/__https:/github.com/cprabha__;!!NEt6yMaO-gk!EWu6slePIXbQAXxNIY594v82RLeQeaDdGmbMvsVamkHU2r89kCeboGYcab0n3i7WPRhrmCkgHZaWkUdIGN-F-pUi$ the error in the gnmi response when using WithTypeSet is a bug that I will fix.

The issue of "only first AS in list 65505 is seen in the routes received in the DUT from ATE" is not necessarily a bug in Ondatra. The question there is whether IxNetwork is configured correctly. Can you check whether IxNetwork is configured to have all three "65505, 65501, 65503" in the AS path segment? If it is, then the Ondatra contract (to configure IxNetwork correctly) has been met.

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/openconfig/featureprofiles/issues/1659*issuecomment-1563596472__;Iw!!NEt6yMaO-gk!EWu6slePIXbQAXxNIY594v82RLeQeaDdGmbMvsVamkHU2r89kCeboGYcab0n3i7WPRhrmCkgHZaWkUdIGJhOCuNB$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AHCEB5V4A25DGE6OMA2TC7LXH7NV7ANCNFSM6AAAAAAYPJBMXQ__;!!NEt6yMaO-gk!EWu6slePIXbQAXxNIY594v82RLeQeaDdGmbMvsVamkHU2r89kCeboGYcab0n3i7WPRhrmCkgHZaWkUdIGLk8-IiR$. You are receiving this because you were mentioned.Message ID: @.**@.>>

Juniper Business Use Only

cprabha commented 1 year ago
asseg
ANISH-GOTTAPU commented 1 year ago

image

Based on Number of As Segments the Number of AS# In Segment needs to be updated. Raised PR https://github.com/openconfig/ondatra/pull/76 to fix the issue

greg-dennis commented 1 year ago

The PR from @ANISH-GOTTAPU fixes the first issue. I am working on the gnmi issue right now. I'll comment back when both fixed are merged into Ondatra.

greg-dennis commented 1 year ago

This is fixed in the latest Ondatra release https://github.com/openconfig/ondatra/releases/tag/v0.1.16 I'll update featureprofiles to require this release

cprabha commented 1 year ago

Thanks Greg.

-Prabha.

From: Greg Dennis @.> Sent: Tuesday, May 30, 2023 10:28 AM To: openconfig/featureprofiles @.> Cc: Prabha Chethan @.>; Mention @.> Subject: Re: [openconfig/featureprofiles] AddASPathSegment(asSeg...) is appending only first AS number (Issue #1659)

[External Email. Be cautious of content]

This is fixed in the latest Ondatra release https://github.com/openconfig/ondatra/releases/tag/v0.1.16https://urldefense.com/v3/__https:/github.com/openconfig/ondatra/releases/tag/v0.1.16__;!!NEt6yMaO-gk!Fz7Gra5tu_RYng3ElV_jEmd3cgo39xUM4ds-Vo7wIw-J-QrMLenUB_1bFf7gpE49uEvP9p1_6w1zkCF79372cI5I$ I'll update featureprofiles to require this release

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/openconfig/featureprofiles/issues/1659*issuecomment-1568806281__;Iw!!NEt6yMaO-gk!Fz7Gra5tu_RYng3ElV_jEmd3cgo39xUM4ds-Vo7wIw-J-QrMLenUB_1bFf7gpE49uEvP9p1_6w1zkCF79z_FWGfR$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AHCEB5VRSCCE5DPVN3GALCTXIYUZPANCNFSM6AAAAAAYPJBMXQ__;!!NEt6yMaO-gk!Fz7Gra5tu_RYng3ElV_jEmd3cgo39xUM4ds-Vo7wIw-J-QrMLenUB_1bFf7gpE49uEvP9p1_6w1zkCF7907QANkR$. You are receiving this because you were mentioned.Message ID: @.**@.>>

Juniper Business Use Only

cprabha commented 1 year ago

Also this path can contain as path as {65501} when SET mode is used. so return value need to be changed from uint32 to different value ?

Routes sent to ATE : 500 100 {600 800 65505}

Error in retrieving. bgp_remove_private_as_test.go:257: Await(t) on ate(10.92.196.126) at /network-instances/network-instance[name=atedst]/protocols/protocol[identifier=BGP][name=0]/bgp/rib/afi-safis/afi-safi[afi-safi-name=IPV4_UNICAST]/ipv4-unicast/neighbors/neighbor[neighbor-address=]/adj-rib-in-pre/routes/route[path-id=0][prefix=]/state/prefix: error receiving gNMI response: failed to append details for elem 0: invalid AS segment member: "600,800,65505"

greg-dennis commented 1 year ago

@cprabha, I'm not clear on why uint is a problem here? 65501 is a valid uint. It seems the problem is that Ixia is now comma-separating the segment members as opposed to separating them with whitespace? I have only seen it whitespace separated in IxNetwork -- maybe it's version dependent? @ANISH-GOTTAPU, the segment members can be either space-separated or comma-separated in the learned info?

greg-dennis commented 1 year ago

@cprabha, could you link me to a draft PR that repro's the issue, too?

ANISH-GOTTAPU commented 1 year ago

@greg-dennis , the segment members within segment are comma-separated and space-separated between as segments. Please find the snapshot

image

image

greg-dennis commented 1 year ago

@ANISH-GOTTAPU, in @cprabha's first comment above the error shows as invalid AS path string: "<500 100> {65501}". That's a space between 500 and 100, and that's what I've see in IxNetwork. Is it sometimes spaces and sometimes commas?

Oh, and what do the brackets [] mean? I know <> is SEQ and {} is SET. Is there a full spec for this syntax you could provide me?

cprabha commented 1 year ago

@greg-dennis I have added script for both "SET" and "SEQ" (currently working fine) . please check.

https://github.com/cprabha/featureprofiles/blob/AsPathSet/feature/bgp/aspath/ate_tests/bgp_remove_private_as_new_test.go

In both Seq and Set mode it is space separated, in Set mode it adds {} to those AS numbers which are added using Set mode.

cprabha commented 1 year ago

@greg-dennis I will check if there is any draft on these and share with you.

greg-dennis commented 1 year ago

@cprabha, sorry what's the status of this? Do you still have an issue here? Any knowledge of the what the brackets mean in "[800 65501 600]"?

cprabha commented 1 year ago

@greg-dennis , There are multiple modes in AS PATH . SET , SEQ , SET confed, SEQ confed. We might see different pattern in each of these modes. I have tested SET and SEQ , SEQ is space separated and SET has pair of {}.

cprabha commented 1 year ago

I could not find any document around this pattern to share.

greg-dennis commented 1 year ago

I found it is comma-separated in SETs and space-separated in SEQs. Example:

<500 100> {65501,65507,65534}

Weird. I'll try to make the parsing logic robust to both.

greg-dennis commented 1 year ago

This is fixed in the latest Ondatra release https://github.com/openconfig/ondatra/releases/tag/v0.1.23

cprabha commented 1 year ago

Thanks @greg-dennis .

cprabha commented 1 year ago

@greg-dennis , looks like these changes are not merged in otg. I am not getting AS numbers appended at OTG. Please could you check.

greg-dennis commented 1 year ago

@cprabha, IxNetwork and OTG gnmi are entirely separate. IxNetwork gNMI is provided by a server that Google wrote and is embedded in Ondatra. I take responsibility for fixing issues there. OTG provides an entirely different yang model. I don't know whether they model the as path segments, but you'd have to take a look at the OTG yang models or ask Keysight. As you may be aware, it's our goal to move entirely to OTG and deprecate IxNetwork.

cprabha commented 1 year ago

@cprabha, IxNetwork and OTG gnmi are entirely separate. IxNetwork gNMI is provided by a server that Google wrote and is embedded in Ondatra. I take responsibility for fixing issues there. OTG provides an entirely different yang model. I don't know whether they model the as path segments, but you'd have to take a look at the OTG yang models or ask Keysight. As you may be aware, it's our goal to move entirely to OTG and deprecate IxNetwork.

Thanks @greg-dennis . I have opened issue under otg for ixia to help on this issue in otg.