Skip to content

Fix ARP issues seen in QOS tests#1420

Merged
neethajohn merged 5 commits intosonic-net:masterfrom
neethajohn:fix_qos_arp_hdrm
Mar 10, 2020
Merged

Fix ARP issues seen in QOS tests#1420
neethajohn merged 5 commits intosonic-net:masterfrom
neethajohn:fix_qos_arp_hdrm

Conversation

@neethajohn
Copy link
Copy Markdown
Contributor

Description of PR

QOS tests were failing because ARP entries weren't populated before the test. Hence packets sent were getting flooded on all ports

Approach

  • Populate the ARP entries for all the ports used by the testcases before the test run.
  • For headroom pool test, the ports are hardcoded for each hwsku. hence ARP populate is run again right before that testcase.

How did you verify/test it?

Ran the testcase on T0 topology and it passes

Signed-off-by: Neetha John <nejo@microsoft.com>
Signed-off-by: Neetha John <nejo@microsoft.com>
Signed-off-by: Neetha John <nejo@microsoft.com>
Signed-off-by: Neetha John <nejo@microsoft.com>
hw_tgt='00:00:00:00:00:00')
send_packet(self, port[1], arpreq_pkt)
index += 1
time.sleep(8)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we use the loop to populate all the time? Why doing things differently in 3 topologies?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is needed only for t0. I will get rid of this check here. There is already a condition check in the yml file itself.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

hw_snd=self.dst_port_mac,
hw_tgt='00:00:00:00:00:00')
send_packet(self, self.dst_port_id, arpreq_pkt)
time.sleep(8)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can this duplicate code handled by class ARPpopulate()?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would need to pass in a lot of variables to the ARPpopulate if i need it to handle this one by calling ARPpopulate from this test.
The other option is to pass in all the variables used by this test for arp population along with the existing set and modify the ARPpopulate class to handle everything.
I prefer the second one if this change is needed.

Copy link
Copy Markdown
Collaborator

@yxieca yxieca left a comment

Choose a reason for hiding this comment

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

Let's move on and clean up later. For duplicate code. Please consider creating a common helper function.

…ology

Signed-off-by: Neetha John <nejo@microsoft.com>
@neethajohn
Copy link
Copy Markdown
Contributor Author

Let's move on and clean up later. For duplicate code. Please consider creating a common helper function.

Sure. will revisit this

@neethajohn neethajohn merged commit 85f0b98 into sonic-net:master Mar 10, 2020
yxieca pushed a commit that referenced this pull request Mar 10, 2020
* Fix ARP issues seen in QOS tests

Signed-off-by: Neetha John <nejo@microsoft.com>
eth_src=self.src_port_mac,
arp_op=1,
ip_snd=self.src_port_ip,
ip_tgt='192.168.0.1',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I' m afraid to hardcoded the ip address as 192.168.0.1 will fail ptf topo. This ip address works only for t0.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is only needed for T0

kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
swss:
* 9376ec6 2021-02-22 | [orchagent] Increase SAI REDIS response timeout to support FW upgrade during init (Mellanox only). (sonic-net#1637) (HEAD -> 202012) [Lior Avramov]
* 6c285f6 2021-02-22 | Updated PFCWD to use single ACL table for PFCWD and MUX (sonic-net#1620) [vmittal-msft]

utilities:
* d2f0e8f 2021-02-24 | [route_check]: Dropped redundant code. (sonic-net#1463) (HEAD -> 202012, github/202012) [Renuka Manavalan]
* 9aaef9b 2021-02-25 | [Mellanox] Add support for SN4600 system (sonic-net#1462) [DavidZagury]
* 60843dd 2021-03-01 | [reboot] Add platform-specific reboot cause update hook (sonic-net#1454) [rkdevi27]
* d9c308c 2021-02-24 | [vlan] Vlan deletion is not allowed when there are members assigned to this VLAN. (sonic-net#1420) [Eran Dahan]
* a72165a 2021-02-25 | [psushow] Add more output columns; Add option to output in JSON format (sonic-net#1416) [Joe LeVeque]

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants