Skip to content

Conversation

@QxBytes
Copy link
Contributor

@QxBytes QxBytes commented Aug 3, 2022

Reason for Change:
Adds SNAT bridge functionality to the native datapath and decouples the OVS-specific SNAT bridge code from the more general SNAT bridge code.

Issue Fixed:
N/A

Requirements:

Notes:

  • The network package was gofumpt so there may be style fixes in the following files not directly related to this feature (See commit "Go fumpt entire network package" for those files).
  • Some code was moved around/renamed/re-used. In particular, snat_linux.go (and most other files) consists of already-existing code that was re-used.
  • The idea is that OVS and Native have their own versions of SNAT/snat route, which call a common set of behaviors in endpoint_snatroute_linux.go, which uses the client to call something in snat_linux.go

QxBytes added 30 commits July 5, 2022 17:01
nw.extIf.Name > eth0 (eth0)
eth0.vlanid > eth0.X (eth0.1)
%s%s hostIfName > vnet (A1veth0)
%s%s-2 contIfName > container (B1veth0)
Moved netnsinterface to consumer package (network).
Removed "Netns" from "NewNetns" and "NewMockNetns" as it is unambiguous.
Changed uintptr to int and casted the int to uintptr when needed later.
Directly instantiating struct because nothing special happens in NewNativeEndpointClient
Forgot to remove uintptr from mocknetns
QxBytes added 14 commits July 26, 2022 09:45
Tests ok, ping ok, wget ok
OVS & Native:
-	Ping between two containers same VM, no packets on bridge
-	Ping between two containers diff VM, no packets on bridge
-	Ping other container not in vnet, no packets on bridge
-	Ping snat to container, packets on bridge
-	Ping container to snat, packets on bridge
-	Tcpdump confirmed on azSnatBr
-	Deletion of containers deletes appropriate interfaces
@rbtr
Copy link
Collaborator

rbtr commented Aug 3, 2022

could you pull the gofumpt of the whole package out into a separate PR? happy to merge that one first

QxBytes added 2 commits August 3, 2022 15:13
eth0.2 makes disable RA look for a folder eth0 and then another sub folder "2". ("eth0/2") However, it should look for a folder named "eth0.2" literally. To solve this, we change the naming scheme to use an underscore instead. (Tests ok)
Copy link
Member

@tamilmani1989 tamilmani1989 left a comment

Choose a reason for hiding this comment

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

i see you updates ovs snat as well.. did you test snat bridge scenarios work in ovs case?

@tamilmani1989 tamilmani1989 merged commit a19e5fb into Azure:master Aug 10, 2022
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