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.

Sign up for GitHub

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

Jump to bottom

Refine the default flow in ARPSpoofGuardTable #5378

Merged
merged 1 commit into from
Aug 22, 2023
Merged

Refine the default flow in ARPSpoofGuardTable #5378

merged 1 commit into from
Aug 22, 2023

Conversation

hongliangl
Copy link
Contributor

The current default flow in ARPSpoofGuardTable forwards packets to ARPResponderTable, which is ineffective in preventing ARP spoofing. To rectify this, the proposed solution is to modify the action of the default flow within ARPSpoofGuardTable to drop the packets.

@hongliangl
Refine the default flow in ARPSpoofGuardTable
f74cc08 
The current default flow in ARPSpoofGuardTable forwards packets
to ARPResponderTable, which is ineffective in preventing ARP
spoofing. To rectify this, the proposed solution is to modify
the action of the default flow within ARPSpoofGuardTable to drop
the packets.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
@hongliangl
Copy link
Contributor Author

@gran-vmv Could you help verify that if this change will affect AntreaIPAM?

@hongliangl hongliangl added area/ovs/openflow Issues or PRs related to Open vSwitch Open Flow. action/backport Indicates a PR that requires backports. labels Aug 10, 2023
@gran-vmv
Copy link
Contributor

@gran-vmv Could you help verify that if this change will affect AntreaIPAM?

I think current flows can handle AntreaIPAM ARP request, but you should run e2e to check.

@hongliangl
Copy link
Contributor Author

/test-all

wenyingd
wenyingd reviewed Aug 14, 2023
View reviewed changes
Copy link
Contributor

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

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

Question: what kind of traffic is expected to go into ARPResponderTable? If the default behavior in ARPSpoofGuardTable is drop, I didn't find a flow explicitly resubmit packets to ARPResponderTable, does it mean flows in responder table are not consumed?

@hongliangl
Copy link
Contributor Author

Question: what kind of traffic is expected to go into ARPResponderTable? If the default behavior in ARPSpoofGuardTable is drop, I didn't find a flow explicitly resubmit packets to ARPResponderTable, does it mean flows in responder table are not consumed?

For example:

1. table=ARPSpoofGuard, priority=200,arp,in_port="antrea-gw0",arp_spa=10.10.0.1,arp_sha=26:4a:01:f6:69:05 actions=resubmit(,ARPResponder)
2. table=ARPSpoofGuard, priority=200,arp,in_port="coredns--24abdc",arp_spa=10.10.0.2,arp_sha=ce:3f:72:79:fa:e5 actions=resubmit(,ARPResponder)
3. table=ARPSpoofGuard, priority=200,arp,in_port="coredns--f86959",arp_spa=10.10.0.3,arp_sha=ca:90:de:6e:d4:06 actions=resubmit(,ARPResponder)
4. table=ARPSpoofGuard, priority=0 actions=drop

The packets matched by flows 1-3 will be consumed in ARPResponderTable.

wenyingd
wenyingd approved these changes Aug 15, 2023
View reviewed changes
Copy link
Contributor

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

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

LGTM

tnqn
tnqn approved these changes Aug 15, 2023
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn
Copy link
Member

tnqn commented Aug 15, 2023

/test-conformance

@gran-vmv
Copy link
Contributor

Why jenkins-flexible-ipam-e2e is not in CI pipelines?

@tnqn
Copy link
Member

tnqn commented Aug 15, 2023

/test-flexible-ipam-e2e

@tnqn
Copy link
Member

tnqn commented Aug 15, 2023

Please check if the failure in jenkins-flexible-ipam-e2e is related

@hongliangl
Copy link
Contributor Author

/test-flexible-ipam-e2e

4 similar comments
@hongliangl
Copy link
Contributor Author

/test-flexible-ipam-e2e

@hongliangl
Copy link
Contributor Author

/test-flexible-ipam-e2e

@hongliangl
Copy link
Contributor Author

/test-flexible-ipam-e2e

@hongliangl
Copy link
Contributor Author

/test-flexible-ipam-e2e

@hongliangl
Copy link
Contributor Author

Flexible IPAM e2e also gets the same failure test cases without any code change. These cases are:

  • TestClusterIPv4/HostNetwork_Endpoints/Connect_to_Service_ClusterIP_from_Pod
  • TestPrometheus/testPrometheusServerAgentMetrics

@hongliangl
Copy link
Contributor Author

hongliangl commented Aug 18, 2023 •
edited
Loading

Flexible IPAM e2e also gets the same failure test cases without any code change. These cases are:

  • TestClusterIPv4/HostNetwork_Endpoints/Connect_to_Service_ClusterIP_from_Pod
  • TestPrometheus/testPrometheusServerAgentMetrics

@tnqn @gran-vmv I think the failure of these two cases is not lated to the current patch. Could we merge this first?

@gran-vmv
Copy link
Contributor

/test-flexible-ipam-e2e

@gran-vmv
Copy link
Contributor

gran-vmv commented Aug 21, 2023 •
edited
Loading

Flexible IPAM e2e also gets the same failure test cases without any code change. These cases are:

  • TestClusterIPv4/HostNetwork_Endpoints/Connect_to_Service_ClusterIP_from_Pod
  • TestPrometheus/testPrometheusServerAgentMetrics

@tnqn @gran-vmv I think the failure of these two cases is not lated to the current patch. Could we merge this first?

It seems some recent changes broke these e2e cases.

@tnqn tnqn merged commit bf3beed into antrea-io:main Aug 22, 2023
46 of 47 checks passed
@hongliangl hongliangl deleted the 20230810-fix-arp-drop branch August 22, 2023 03:25
This was referenced Aug 22, 2023
Merged
Merged
@hongliangl hongliangl mentioned this pull request Aug 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wenyingd wenyingd wenyingd approved these changes

@tnqn tnqn tnqn approved these changes

@gran-vmv gran-vmv Awaiting requested review from gran-vmv

Assignees
No one assigned
Labels
action/backport Indicates a PR that requires backports. area/ovs/openflow Issues or PRs related to Open vSwitch Open Flow.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hongliangl @gran-vmv @tnqn @wenyingd