Skip to content

Added new tool for finding packet in buffer of PTF#3525

Merged
wangxin merged 3 commits intosonic-net:masterfrom
OleksandrKozodoi:add_filter_pkt_in_buffer_tool
Jul 8, 2021
Merged

Added new tool for finding packet in buffer of PTF#3525
wangxin merged 3 commits intosonic-net:masterfrom
OleksandrKozodoi:add_filter_pkt_in_buffer_tool

Conversation

@OleksandrKozodoi
Copy link
Copy Markdown
Contributor

@OleksandrKozodoi OleksandrKozodoi commented May 24, 2021

Signed-off-by: Oleksandr Kozodoi oleksandrx.kozodoi@intel.com

Description of PR

Summary:

  • Added realization of the method for finding packet in the buffer of PTF.
  • Added realization of the method for finding the difference between the expected packet and the received packet in the buffer of PTF.
  • Added realization of the function for converting scapy packet to dictionary.
  • Added realization of the method for printing the packet structure without ignored fields.
  • Added HLD

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

The investigation shows that testutils tool doesn't support verifying packets on the PTF in specific cases, but packets are verified by using tcpdump tool. So was added new method for sniffing packets inside ptfadapter buffer.

How did you do it?

Added realization of the method for finding packets in the buffer of PTF.

How did you verify/test it?

This approach is used in new sub-ports test cases.

Any platform specific information?

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

Documentation

Link

Signed-off-by: Oleksandr Kozodoi <oleksandrx.kozodoi@intel.com>
@OleksandrKozodoi OleksandrKozodoi requested a review from a team as a code owner May 24, 2021 16:54
@OleksandrKozodoi
Copy link
Copy Markdown
Contributor Author

@yxieca @wangxin Could you please review?

Signed-off-by: Oleksandr Kozodoi <oleksandrx.kozodoi@intel.com>
@OleksandrKozodoi
Copy link
Copy Markdown
Contributor Author

@bingwang-ms Can you take a look? Thank you in advance.

@SavchukRomanLv
Copy link
Copy Markdown

@wangxin @lguohan can you please take a look?

@OleksandrKozodoi
Copy link
Copy Markdown
Contributor Author

@yxieca @wangxin @bingwang-ms Could you please review? Thank you in advance.

@wangxin
Copy link
Copy Markdown
Collaborator

wangxin commented Jun 8, 2021

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

```
##### Received packet
```
>>> filter.show_packet(pkt_type='received')
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.

Could there be multiple received packets? This is to list all the received packets in the buffer, right?

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.

show_packet(pkt_type='received') doesn't show all received packets. The method shows only received packet if this packet has fields from the match_fields and packet equals the expected packet.

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.

Same concern of detecting duplicated packets here.

self.received_pkt = self.__find_pkt_in_buffer()

if self.received_pkt:
self.__ignore_fields()
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.

This will update self.masked_exp_pkt in place. Maybe it is better to call self.__ignore_fields() in __init__.

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.

Good idea. Thanks!

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. Thanks!

except KeyError:
break
else:
return Ether(pkt[0])
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.

Only the first packet matches the exp_pkt will be returned? What if there are multiple packets in buffer match the exp_pkt?

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.

The method returns the first packet which matches the exp_pkt. Do we need to return multiple packets?

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.

I think yes, then duplicated packets can be detected.

Signed-off-by: Oleksandr Kozodoi <oleksandrx.kozodoi@intel.com>
@wangxin wangxin merged commit 2b2628a into sonic-net:master Jul 8, 2021
vmittal-msft pushed a commit to vmittal-msft/sonic-mgmt that referenced this pull request Sep 28, 2021
Summary:
* Added realization of the method for finding packet in the buffer of PTF.
* Added realization of the method for finding the difference between the expected packet and the received packet in the buffer of PTF.
* Added realization of the function for converting scapy packet to dictionary.
* Added realization of the method for printing the packet structure without ignored fields.
* Added HLD

What is the motivation for this PR?
The investigation shows that testutils tool doesn't support verifying packets on the PTF in specific cases, but packets are verified by using tcpdump tool. So was added new method for sniffing packets inside ptfadapter buffer.

How did you do it?
Added realization of the method for finding packets in the buffer of PTF.

How did you verify/test it?
This approach is used in new sub-ports test cases.

Signed-off-by: Oleksandr Kozodoi <oleksandrx.kozodoi@intel.com>
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.

3 participants