[dir_bcast] Fix issue of dir_bcast testing by wangxin · Pull Request #771 · sonic-net/sonic-mgmt

wangxin · 2019-01-14T04:41:18Z

[dir_bcast] Fix issue of dir_bcast testing

If DHCP relay server is configured on DUT, the injected packet would be
forwarded to the DHCP servers. This fix replaced the injected BOOTP UDP packet
with ordinary IP packet to ensure that it is broadcasted to all
interfaces of the VLAN.

Signed-off-by: Xin Wang xinw@mellanox.com

Description of PR

Summary:

If DHCP relay server is configured, the injected BOOTP packet would be unicast to the configured DHCP servers and this test case would fail. This fix replaced the injected BOOTP packet with ordinary IP packet. Then the injected packet will be broadcasted to all the destination VLAN interfaces.

Type of change

[*] Bug fix
[] Testbed and Framework(new/improvement)
[] Test case(new/improvement)

Approach

How did you do it?

Replace the injected BOOTP packet with an ordinary IP packet.

How did you verify/test it?

Tested on Mellanox platform running t0 topology. Used the latest image from master branch.

Any platform specific information?

N/A

Supported testbed topology if it's a new test case?

Documentation

If DHCP relay server is configured, the injected packet would be forwarded to the DHCP servers. Replace the injected BOOTP UDP packet with ordinary IP packet to ensure that it is broadcasted to all interfaces of the VLAN. Signed-off-by: Xin Wang <xinw@mellanox.com>

Change the source IP address of injected packets to 1.1.1.1 Signed-off-by: Xin Wang <xinw@mellanox.com>

prsunny · 2019-01-16T17:23:55Z

The intention of this test is to verify mainly the DHCP directed broadcast. The packet forwarding should happen in ASIC for the DHCP packet that is being sent to directed broadcast address which in this case would get forwarded to all member ports of Vlan. I would like to understand, how does this get bypass and forward only to the configured server?

prsunny · 2019-01-17T02:28:39Z

@liat-grozovik, Itai - Is this behavior seen coz current MLNX SAI has L2 DHCP trap instead of L3 DHCP trap? Once it is fixed in SAI, this could be retested to verify the behavior.

wangxin · 2019-01-17T14:37:46Z

@prsunny
Could you please elaborate what is the expected behavior if:

inject a DHCP packekt, destined to subnet broadcast address of VLAN interface
inject an ordinary IP packet, destined to subnet broadcast address of VLAN interface

For both scenarios, should the injected packet be forwarded to all members of VLAN?

prsunny · 2019-01-17T16:49:28Z

@wangxin , yes for both scenarios, the packet must be forwarded to all members of Vlan. That's my point as well. So why do you have to change DHCP to ordinary IP packet?

wangxin · 2019-01-18T03:08:15Z

@prsunny ,
The DUT device has DHCP servers configured for the VLAN. When DHCP directed broadcast packets are received by this VLAN interface, IMO, it makes sense to unicast these DHCP packets to the configured DHCP servers. Otherwise, what's the purpose of these DHCP servers?

wangxin · 2019-01-18T03:32:39Z

Hi, @prsunny, I just got confirmation from Itai. The new behavior will be to trap DHCP at L3. Only broadcast DHCP packets will be trapped. Directed broadcast DHCP packets will be handled as ordinary IP packet. I will change this script to cover both directed broadcast DHCP and non-DHCP IP packets.

lguohan

based on discussion in the thread.

Both directed broadcast DHCP and non-DHCP packets should be broadcasted to all members of VLAN. Add code for checking directed broadcast of DHCP packets along with non-DHCP packets. Signed-off-by: Xin Wang <xinw@mellanox.com>

wangxin · 2019-02-20T07:35:47Z

Committed another fix. Added checking of DHCP packets. What the script does now:

Inject non-DHCP direct broadcast packets, verify that the packet is broadcasted on all VLAN members
Inject DHCP direct broadcast packets, verify that the packet is broadcasted on all VLAN members.
Tested on latest master build and passed. Please kindly help review the changes. Thanks!

liat-grozovik · 2019-02-25T07:22:04Z

@lguohan as all comments were addresses, can you please help review and merge?

prsunny · 2019-03-06T17:05:18Z

ansible/roles/test/files/ptftests/dir_bcast_test.py

+
+    #---------------------------------------------------------------------
+
+    def check_bootp_dir_bcast(self, dst_bcast_ip, dst_port_list):


To me it looks like most of the functionality except the packet is same for both functions. We could create a single function for common code and use these functions only to create the packet. Approving from my side!

liat-grozovik · 2019-03-13T17:04:36Z

@lguohan please review fixes following your comments.

…ty related errors (sonic-net#20596) (sonic-net#771) Cherry-pick sonic-net#20596 What is the motivation for this PR? Add regex to skip sai_query_attribute_capability related loganalyzer errors How did you do it? Added 2 regex to loganalyzer_common_ignore.txt file to ignore sai_query_attribute_enum_values_capability "rv=-8", ignoring non-functional error in swss PR pipeline till Fix switch capabilities enum query with metadata validation sonic-swss#3866 is merged sai_query_attribute_capability "error -2", ignoring till it gets fixed from SAI, CS00012422823 is raised How did you verify/test it? Tested on Arista-7050X3 running dualtor-aa topology

This update includes the following changes > [syncd armhf] Fix syncd crash when running community test suites (sonic-net#777) > Revert "[tests]:Add unittest for MACsec on p2p establishment (sonic-net#771)" > [tests]:Add unittest for MACsec on p2p establishment (sonic-net#771) > [tests] Enable azure pipeline make check to respect unittests (sonic-net#760)

Xin Wang added 2 commits January 14, 2019 11:32

[dir_bcast] Use an obviously unused source IP address 1.1.1.1

5eeff40

Change the source IP address of injected packets to 1.1.1.1 Signed-off-by: Xin Wang <xinw@mellanox.com>

liat-grozovik approved these changes Jan 16, 2019

View reviewed changes

pavel-shirshov requested a review from prsunny January 17, 2019 00:57

lguohan requested changes Jan 18, 2019

View reviewed changes

[dir_bcast] Add checking directed broadcast of DHCP packets

99bc316

Both directed broadcast DHCP and non-DHCP packets should be broadcasted to all members of VLAN. Add code for checking directed broadcast of DHCP packets along with non-DHCP packets. Signed-off-by: Xin Wang <xinw@mellanox.com>

liat-grozovik requested a review from lguohan February 28, 2019 08:31

prsunny reviewed Mar 6, 2019

View reviewed changes

prsunny approved these changes Mar 6, 2019

View reviewed changes

lguohan approved these changes Mar 22, 2019

View reviewed changes

lguohan merged commit abe098f into sonic-net:master Mar 22, 2019

wangxin deleted the dir-bcast branch May 24, 2019 03:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[dir_bcast] Fix issue of dir_bcast testing#771

[dir_bcast] Fix issue of dir_bcast testing#771
lguohan merged 3 commits intosonic-net:masterfrom
wangxin:dir-bcast

wangxin commented Jan 14, 2019

Uh oh!

prsunny commented Jan 16, 2019

Uh oh!

prsunny commented Jan 17, 2019

Uh oh!

wangxin commented Jan 17, 2019

Uh oh!

prsunny commented Jan 17, 2019

Uh oh!

wangxin commented Jan 18, 2019

Uh oh!

wangxin commented Jan 18, 2019

Uh oh!

lguohan left a comment

Uh oh!

wangxin commented Feb 20, 2019

Uh oh!

liat-grozovik commented Feb 25, 2019

Uh oh!

prsunny Mar 6, 2019

Uh oh!

liat-grozovik commented Mar 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants


		#---------------------------------------------------------------------

		def check_bootp_dir_bcast(self, dst_bcast_ip, dst_port_list):

Conversation

wangxin commented Jan 14, 2019

Description of PR

Type of change

Approach

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

prsunny commented Jan 16, 2019

Uh oh!

prsunny commented Jan 17, 2019

Uh oh!

wangxin commented Jan 17, 2019

Uh oh!

prsunny commented Jan 17, 2019

Uh oh!

wangxin commented Jan 18, 2019

Uh oh!

wangxin commented Jan 18, 2019

Uh oh!

lguohan left a comment

Choose a reason for hiding this comment

Uh oh!

wangxin commented Feb 20, 2019

Uh oh!

liat-grozovik commented Feb 25, 2019

Uh oh!

prsunny Mar 6, 2019

Choose a reason for hiding this comment

Uh oh!

liat-grozovik commented Mar 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants