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

Update packages and pointers to use SAI1.6 headers. #4597

Merged
merged 23 commits into from
May 30, 2020
Merged

Update packages and pointers to use SAI1.6 headers. #4597

merged 23 commits into from
May 30, 2020

Conversation

smaheshm
Copy link
Contributor

@smaheshm smaheshm commented May 14, 2020

- Why I did it

Update sonic-sairedis pointer to use SAI1.6 headers

ebdf4ff (HEAD -> master, origin/master, origin/HEAD) [build]: support lgtm build for python library (#615)                                                                            
c819dc7 Change log level for not implemented port attributes and skipped mac learning (#610)                                                                                          
d12646c [MultiDB] use new multiDB API/cmd in new added codes (#611)                                                                                                                   
88a1982 [syncd] Use steady clock for TimerWatchdog (#613)                                                                                                                             
e50cf66 updating sonic-sairedis submodule to include SAI 1.6 headers                                                                                                                  
fea8e05 [py] Add sairedis python module skeleton (#607)                                                                                                                               
1a61519 [saiplayer] Fix notification pointers (#604)                                                                                                                                  
a77fc0a [sairedis] [meta] Support connect to existing switch (#602)                                                                                                                   
0b43cab [vs] Make default port admin state to false (#603)                                

- How I did it

  • Updated sonic-sairedis submodule.
  • Updated pointer brcm-sai debian package
  • Merged changes from Mellanox to use SAI1.6

- How to verify it

  • Built "target/sonic-broadcom.bin", loading and verify basic connective on lab switch

- Description for the changelog

  • Updated sonic-sairedis submodule.
  • Updated pointer brcm-sai debian package
  • Merged changes from Mellanox to use SAI1.6

- A picture of a cute animal (not mandatory but encouraged)

image

@lguohan
Copy link
Collaborator

lguohan commented May 14, 2020

build failures

@smaheshm
Copy link
Contributor Author

retest broadcom please

smaheshm and others added 4 commits May 15, 2020 01:14
… 1.16.3 (API:v1.6)

Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com>
[Mellanox] Update SDK to 4.4.0914 and FW to xx.2007.1112 to match SAI…
@akokhan
Copy link
Contributor

akokhan commented May 20, 2020

retest vs please

@akokhan
Copy link
Contributor

akokhan commented May 20, 2020

retest mellanox please

@lguohan
Copy link
Collaborator

lguohan commented May 21, 2020

why are mellanox and vs failing?

@akokhan
Copy link
Contributor

akokhan commented May 22, 2020

retest vs please

@akokhan
Copy link
Contributor

akokhan commented May 22, 2020

retest mellanox please

@lguohan
Copy link
Collaborator

lguohan commented May 27, 2020

@smaheshm, what is the issue here?

@smaheshm
Copy link
Contributor Author

@smaheshm, what is the issue here?

BGP GR livelock test fails due to veth interface is in 'DOWN' state.

For some reason 'veth' I/F (eth1, eth2) goes down once the docker VS comes up. This doesn't happen in earlier release.

There's a netlink down event generated for eth1 (and eth2) once VS image comes up but I don't see the any message in syslog to shut down the eth1 I/F.

Added an explicit i/f up command in the BGP GR livelock test.

@rlhui
Copy link
Contributor

rlhui commented May 28, 2020

@lguohan , @abdosi , please help review this PR? Thanks.

Copy link
Contributor

@abdosi abdosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smaheshm, what is the issue here?

BGP GR livelock test fails due to veth interface is in 'DOWN' state.

For some reason 'veth' I/F (eth1, eth2) goes down once the docker VS comes up. This doesn't happen in earlier release.

There's a netlink down event generated for eth1 (and eth2) once VS image comes up but I don't see the any message in syslog to shut down the eth1 I/F.

Added an explicit i/f up command in the BGP GR livelock test.

@kcudnik is this happening because of some change in vslib/src/SwitchStateBaseHostif.cpp.
There is some change in logic of ifup.

Can you check.

@@ -15,6 +15,8 @@ def test_InvalidNexthop(dvs, testlog):
dvs.servers[0].runcmd("ip addr add fc00::2/126 dev eth0")
dvs.servers[0].runcmd("ifconfig eth0 up")

dvs.runcmd("ip link set dev eth1 up")
dvs.runcmd("ip link set dev eth2 up")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this happening because of change in vslib/src/SwitchStateBaseHostif.cpp ? I see this file has some change in logic

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abdosi is correct. this is due to this pr. sonic-net/sonic-sairedis@0b43cab

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should do something similiar to this. sonic-net/sonic-swss@aa116f4

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the correct fix should be line 13 should be changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smaheshm , what's the resolution of this? Are these comments fully resolved? Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kcudnik, did you get a chance to check this one? Thanks.

@@ -20,6 +20,8 @@ def test_bounce(dvs, testlog):
dvs.servers[1].runcmd("ip addr add 10.0.0.3/31 dev eth0")
dvs.servers[1].runcmd("ifconfig eth0 up")

dvs.runcmd("ip link set dev eth1 up")
dvs.runcmd("ip link set dev eth2 up")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above.

@lguohan
Copy link
Collaborator

lguohan commented May 29, 2020

who is going to fix the mellanox build issue?

@smaheshm
Copy link
Contributor Author

retest vsimage please

1 similar comment
@smaheshm
Copy link
Contributor Author

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented May 29, 2020

can you update your commit message to list all the commit message for the submodule update?

@smaheshm smaheshm merged commit fb6916f into sonic-net:master May 30, 2020
@lguohan
Copy link
Collaborator

lguohan commented May 30, 2020

frankly speaking, the commit message in the pr fb6916f is really confusing, lots of useless immediate information. Please be careful about the commit message. You need to craft a clear commit message when you merge it.

@smaheshm
Copy link
Contributor Author

smaheshm commented Jun 2, 2020

frankly speaking, the commit message in the pr fb6916f is really confusing, lots of useless immediate information. Please be careful about the commit message. You need to craft a clear commit message when you merge it.

Got it, will edit the commit message before merging.

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.

5 participants