Skip to content

Commit

Permalink
Provide generic API for OVS register match/set value
Browse files Browse the repository at this point in the history
1. Use RegMark/RegField in OpenFlow Match or Load actions instead of passing the register ID and range.
2. The usage for OVS registers(including xxreg) should be predefined in regmarks.go.
3. Use reg0[0..3] to indicate the source of packet, and release reg0[4..15] for other usages.

Signed-off-by: wenyingd <wenyingd@vmware.com>
  • Loading branch information
wenyingd committed Aug 5, 2021
1 parent ae98a19 commit cad46af
Show file tree
Hide file tree
Showing 22 changed files with 748 additions and 385 deletions.
12 changes: 6 additions & 6 deletions docs/design/ovs-pipeline.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ that Node and `<BRIDGE_NAME>` is the name of the bridge created by Antrea
We use 2 32-bit OVS registers to carry information throughout the pipeline:

* reg0 (NXM_NX_REG0):
- bits [0..15] are used to store the traffic source (from tunnel: 0, from
- bits [0..3] are used to store the traffic source (from tunnel: 0, from
local gateway: 1, from local Pod: 2). It is set in the [ClassifierTable].
- bit 16 is used to indicate whether the destination MAC address of a packet
is "known", i.e. corresponds to an entry in [L2ForwardingCalcTable], which
Expand Down Expand Up @@ -621,8 +621,8 @@ be SNAT'd with a SNAT IP configured on the Node. In the latter case, the flow
also rewrites the destination MAC to the local gateway interface MAC.

```text
table=70, priority=190,ip,reg0=0x2/0xffff actions=goto_table:71
table=70, priority=190,ip,reg0=0/0xffff actions=mod_dl_dst:e2:e5:a4:9b:1c:b1,goto_table:71
table=70, priority=190,ip,reg0=0x2/0xf actions=goto_table:71
table=70, priority=190,ip,reg0=0/0xf actions=mod_dl_dst:e2:e5:a4:9b:1c:b1,goto_table:71
```

### SNATTable (71)
Expand All @@ -639,7 +639,7 @@ Egress applied, to [L2ForwardingCalcTable]. Such traffic will be SNAT'd with
the default SNAT IP (by an iptables masquerade rule).

```text
table=71, priority=190,ct_state=+new+trk,ip,reg0=0/0xffff actions=drop
table=71, priority=190,ct_state=+new+trk,ip,reg0=0/0xf actions=drop
table=71, priority=0 actions=goto_table:80
```

Expand Down Expand Up @@ -685,7 +685,7 @@ IP stack should have already decremented TTL if that is needed.
If you dump the flows for this table, you should see flows like the following:

```text
1. table=72, priority=210,ip,reg0=0x1/0xffff, actions=goto_table:80
1. table=72, priority=210,ip,reg0=0x1/0xf, actions=goto_table:80
2. table=72, priority=200,ip, actions=dec_ttl,goto_table:80
3. table=72, priority=0, actions=goto_table:80
```
Expand Down Expand Up @@ -884,7 +884,7 @@ which are not dropped because of Network Policies. If you dump the flows for thi
table, you should see something like this:

```text
1. table=105, priority=200,ct_state=+new+trk,ip,reg0=0x1/0xffff actions=ct(commit,table=110,zone=65520,exec(load:0x20->NXM_NX_CT_MARK[]))
1. table=105, priority=200,ct_state=+new+trk,ip,reg0=0x1/0xf actions=ct(commit,table=110,zone=65520,exec(load:0x20->NXM_NX_CT_MARK[]))
2. table=105, priority=190,ct_state=+new+trk,ip actions=ct(commit,table=110,zone=65520)
3. table=105, priority=0 actions=goto_table:110
```
Expand Down
33 changes: 18 additions & 15 deletions pkg/agent/controller/networkpolicy/packetin.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,27 @@ func (c *Controller) HandlePacketIn(pktIn *ofctrl.PacketIn) error {

matches := pktIn.GetMatches()
// Get custom reasons in this packet-in.
match := getMatchRegField(matches, uint32(openflow.CustomReasonMarkReg))
customReasons, err := getInfoInReg(match, openflow.CustomReasonMarkRange.ToNXRange())
match := getMatchRegField(matches, openflow.CustomReasonField)
customReasons, err := getInfoInReg(match, openflow.CustomReasonField.GetRange().ToNXRange())
if err != nil {
return fmt.Errorf("received error while unloading customReason from reg: %v", err)
}

// Use reasons to choose operations.
if customReasons&openflow.CustomReasonLogging == openflow.CustomReasonLogging {
var checkCustomReason = func(customReasonMark *binding.RegMark) bool {
return customReasons&customReasonMark.GetValue() == customReasonMark.GetValue()
}
if checkCustomReason(openflow.CustomReasonLoggingMark) {
if err := c.logPacket(pktIn); err != nil {
return err
}
}
if customReasons&openflow.CustomReasonReject == openflow.CustomReasonReject {
if checkCustomReason(openflow.CustomReasonRejectMark) {
if err := c.rejectRequest(pktIn); err != nil {
return err
}
}
if customReasons&openflow.CustomReasonDeny == openflow.CustomReasonDeny {
if checkCustomReason(openflow.CustomReasonDenyMark) {
if err := c.storeDenyConnection(pktIn); err != nil {
return err
}
Expand Down Expand Up @@ -152,27 +155,27 @@ func (c *Controller) logPacket(pktIn *ofctrl.PacketIn) error {
return nil
}

// getMatchRegField returns match to the regNum register.
func getMatchRegField(matchers *ofctrl.Matchers, regNum uint32) *ofctrl.MatchField {
return matchers.GetMatchByName(fmt.Sprintf("NXM_NX_REG%d", regNum))
// getMatchRegField returns the match register according to the specified RegField.
func getMatchRegField(matchers *ofctrl.Matchers, field *binding.RegField) *ofctrl.MatchField {
return matchers.GetMatchByName(field.GetNXFieldName())
}

// getMatch receives ofctrl matchers and table id, match field.
// Modifies match field to Ingress/Egress register based on tableID.
func getMatch(matchers *ofctrl.Matchers, tableID binding.TableIDType, disposition uint32) *ofctrl.MatchField {
// Get match from CNPDenyConjIDReg if disposition is not allow.
if disposition != openflow.DispositionAllow {
return getMatchRegField(matchers, uint32(openflow.CNPDenyConjIDReg))
return getMatchRegField(matchers, openflow.CNPDenyConjIDField)
}
// Get match from ingress/egress reg if disposition is allow
for _, table := range append(openflow.GetAntreaPolicyEgressTables(), openflow.EgressRuleTable) {
if tableID == table {
return getMatchRegField(matchers, uint32(openflow.EgressReg))
return getMatchRegField(matchers, openflow.TFEgressConjIDField)
}
}
for _, table := range append(openflow.GetAntreaPolicyIngressTables(), openflow.IngressRuleTable) {
if tableID == table {
return getMatchRegField(matchers, uint32(openflow.IngressReg))
return getMatchRegField(matchers, openflow.TFIngressConjIDField)
}
}
return nil
Expand All @@ -199,8 +202,8 @@ func getNetworkPolicyInfo(pktIn *ofctrl.PacketIn, c *Controller, ob *logInfo) er
ob.tableName = openflow.GetFlowTableName(tableID)

// Get disposition Allow or Drop
match = getMatchRegField(matchers, uint32(openflow.DispositionMarkReg))
info, err := getInfoInReg(match, openflow.APDispositionMarkRange.ToNXRange())
match = getMatchRegField(matchers, openflow.APDispositionField)
info, err := getInfoInReg(match, openflow.APDispositionField.GetRange().ToNXRange())
if err != nil {
return fmt.Errorf("received error while unloading disposition from reg: %v", err)
}
Expand Down Expand Up @@ -372,8 +375,8 @@ func (c *Controller) storeDenyConnection(pktIn *ofctrl.PacketIn) error {
// Get table ID
tableID := binding.TableIDType(pktIn.TableId)
// Get disposition Allow, Drop or Reject
match = getMatchRegField(matchers, uint32(openflow.DispositionMarkReg))
id, err := getInfoInReg(match, openflow.APDispositionMarkRange.ToNXRange())
match = getMatchRegField(matchers, openflow.APDispositionField)
id, err := getInfoInReg(match, openflow.APDispositionField.GetRange().ToNXRange())
if err != nil {
return fmt.Errorf("error when getting disposition from reg: %v", err)
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/agent/controller/traceflow/packetin.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func (c *Controller) parsePacketIn(pktIn *ofctrl.PacketIn) (*crdv1alpha1.Tracefl
obs = append(obs, *ob)
}
// Collect egress conjunctionID and get NetworkPolicy from cache.
if match := getMatchRegField(matchers, uint32(openflow.EgressReg)); match != nil {
if match := getMatchRegField(matchers, openflow.TFEgressConjIDField); match != nil {
egressInfo, err := getRegValue(match, nil)
if err != nil {
return nil, nil, nil, err
Expand All @@ -191,7 +191,7 @@ func (c *Controller) parsePacketIn(pktIn *ofctrl.PacketIn) (*crdv1alpha1.Tracefl
}

// Collect ingress conjunctionID and get NetworkPolicy from cache.
if match := getMatchRegField(matchers, uint32(openflow.IngressReg)); match != nil {
if match := getMatchRegField(matchers, openflow.TFIngressConjIDField); match != nil {
ingressInfo, err := getRegValue(match, nil)
if err != nil {
return nil, nil, nil, err
Expand All @@ -207,7 +207,7 @@ func (c *Controller) parsePacketIn(pktIn *ofctrl.PacketIn) (*crdv1alpha1.Tracefl
// Get drop table.
if tableID == uint8(openflow.EgressMetricTable) || tableID == uint8(openflow.IngressMetricTable) {
ob := getNetworkPolicyObservation(tableID, tableID == uint8(openflow.IngressMetricTable))
if match := getMatchRegField(matchers, uint32(openflow.CNPDenyConjIDReg)); match != nil {
if match := getMatchRegField(matchers, openflow.CNPDenyConjIDField); match != nil {
notAllowConjInfo, err := getRegValue(match, nil)
if err != nil {
return nil, nil, nil, err
Expand Down Expand Up @@ -239,7 +239,7 @@ func (c *Controller) parsePacketIn(pktIn *ofctrl.PacketIn) (*crdv1alpha1.Tracefl
}
}
var outputPort uint32
if match := getMatchRegField(matchers, uint32(openflow.PortCacheReg)); match != nil {
if match := getMatchRegField(matchers, openflow.TargetOFPortField); match != nil {
outputPort, err = getRegValue(match, nil)
if err != nil {
return nil, nil, nil, err
Expand Down Expand Up @@ -271,8 +271,8 @@ func (c *Controller) parsePacketIn(pktIn *ofctrl.PacketIn) (*crdv1alpha1.Tracefl
return tf, &nodeResult, capturedPacket, nil
}

func getMatchRegField(matchers *ofctrl.Matchers, regNum uint32) *ofctrl.MatchField {
return matchers.GetMatchByName(fmt.Sprintf("NXM_NX_REG%d", regNum))
func getMatchRegField(matchers *ofctrl.Matchers, field *binding.RegField) *ofctrl.MatchField {
return matchers.GetMatchByName(field.GetNXFieldName())
}

func getMatchTunnelDstField(matchers *ofctrl.Matchers, isIPv6 bool) *ofctrl.MatchField {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (cs *ConntrackConnectionStore) AddOrUpdateConn(conn *flowexporter.Connectio
klog.V(4).Infof("Antrea flow updated: %v", existingConn)
} else {
cs.fillPodInfo(conn)
if conn.Mark == openflow.ServiceCTMark {
if conn.Mark == openflow.ServiceCTMark.GetValue() {
clusterIP := conn.DestinationServiceAddress.String()
svcPort := conn.DestinationServicePort
protocol, err := lookupServiceProtocol(conn.FlowKey.Protocol)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func TestConntrackConnectionStore_AddOrUpdateConn(t *testing.T) {
StartTime: refTime.Add(-(time.Second * 50)),
StopTime: refTime,
FlowKey: tuple4,
Mark: openflow.ServiceCTMark,
Mark: openflow.ServiceCTMark.GetValue(),
IsPresent: true,
}
// Create copy of old conntrack flow for testing purposes.
Expand Down
2 changes: 1 addition & 1 deletion pkg/agent/flowexporter/exporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ func (exp *flowExporter) findFlowType(conn flowexporter.Connection) uint8 {
return 0
}
if exp.nodeRouteController.IPInPodSubnets(conn.FlowKey.SourceAddress) {
if conn.Mark == openflow.ServiceCTMark || exp.nodeRouteController.IPInPodSubnets(conn.FlowKey.DestinationAddress) {
if conn.Mark == openflow.ServiceCTMark.GetValue() || exp.nodeRouteController.IPInPodSubnets(conn.FlowKey.DestinationAddress) {
if conn.SourcePodName == "" || conn.DestinationPodName == "" {
return ipfixregistry.FlowTypeInterNode
}
Expand Down
6 changes: 2 additions & 4 deletions pkg/agent/openflow/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1021,8 +1021,7 @@ func (c *client) SendTCPPacketOut(

// Reject response packet should bypass ConnTrack
if isReject {
name := fmt.Sprintf("%s%d", binding.NxmFieldReg, marksReg)
packetOutBuilder = packetOutBuilder.AddLoadAction(name, uint64(CustomReasonReject), CustomReasonMarkRange)
packetOutBuilder = packetOutBuilder.AddLoadRegMark(CustomReasonRejectMark)
}

packetOutObj := packetOutBuilder.Done()
Expand Down Expand Up @@ -1060,8 +1059,7 @@ func (c *client) SendICMPPacketOut(

// Reject response packet should bypass ConnTrack
if isReject {
name := fmt.Sprintf("%s%d", binding.NxmFieldReg, marksReg)
packetOutBuilder = packetOutBuilder.AddLoadAction(name, uint64(CustomReasonReject), CustomReasonMarkRange)
packetOutBuilder = packetOutBuilder.AddLoadRegMark(CustomReasonRejectMark)
}

packetOutObj := packetOutBuilder.Done()
Expand Down
36 changes: 18 additions & 18 deletions pkg/agent/openflow/network_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,12 +336,12 @@ func TestBatchInstallPolicyRuleFlows(t *testing.T) {
return []binding.Flow{
c.pipeline[EgressRuleTable].BuildFlow(priorityLow).Cookie(cookiePolicy).
MatchProtocol(binding.ProtocolIP).MatchConjID(10).
Action().LoadRegRange(int(EgressReg), 10, binding.Range{0, 31}).
Action().CT(true, EgressMetricTable, CtZone).LoadToLabelRange(10, &metricEgressRuleIDRange).CTDone().Done(),
Action().LoadToRegField(TFEgressConjIDField, 10).
Action().CT(true, EgressMetricTable, CtZone).LoadToLabelRange(10, metricEgressRuleIDRange).CTDone().Done(),
c.pipeline[EgressRuleTable].BuildFlow(priorityLow).Cookie(cookiePolicy).
MatchProtocol(binding.ProtocolIP).MatchConjID(11).
Action().LoadRegRange(int(EgressReg), 11, binding.Range{0, 31}).
Action().CT(true, EgressMetricTable, CtZone).LoadToLabelRange(11, &metricEgressRuleIDRange).CTDone().Done(),
Action().LoadToRegField(TFEgressConjIDField, 11).
Action().CT(true, EgressMetricTable, CtZone).LoadToLabelRange(11, metricEgressRuleIDRange).CTDone().Done(),
c.pipeline[EgressRuleTable].BuildFlow(priorityNormal).Cookie(cookiePolicy).
MatchProtocol(binding.ProtocolIP).MatchSrcIP(net.ParseIP("192.168.1.40")).
Action().Conjunction(10, 1, 2).
Expand Down Expand Up @@ -441,17 +441,17 @@ func TestBatchInstallPolicyRuleFlows(t *testing.T) {
return []binding.Flow{
c.pipeline[AntreaPolicyIngressRuleTable].BuildFlow(priority100).Cookie(cookiePolicy).
MatchProtocol(binding.ProtocolIP).MatchConjID(10).
Action().LoadRegRange(int(IngressReg), 10, binding.Range{0, 31}).
Action().CT(true, IngressMetricTable, CtZone).LoadToLabelRange(10, &metricIngressRuleIDRange).CTDone().Done(),
Action().LoadToRegField(TFIngressConjIDField, 10).
Action().CT(true, IngressMetricTable, CtZone).LoadToLabelRange(10, metricIngressRuleIDRange).CTDone().Done(),
c.pipeline[AntreaPolicyIngressRuleTable].BuildFlow(priority100).Cookie(cookiePolicy).
MatchConjID(11).
Action().LoadRegRange(int(CNPDenyConjIDReg), 11, binding.Range{0, 31}).
Action().LoadRegRange(int(marksReg), cnpDenyMark, cnpDenyMarkRange).
Action().LoadToRegField(CNPDenyConjIDField, 11).
Action().LoadRegMark(CnpDenyMark).
Action().GotoTable(IngressMetricTable).Done(),
c.pipeline[AntreaPolicyIngressRuleTable].BuildFlow(priority200).Cookie(cookiePolicy).
MatchConjID(12).
Action().LoadRegRange(int(CNPDenyConjIDReg), 12, binding.Range{0, 31}).
Action().LoadRegRange(int(marksReg), cnpDenyMark, cnpDenyMarkRange).
Action().LoadToRegField(CNPDenyConjIDField, 12).
Action().LoadRegMark(CnpDenyMark).
Action().GotoTable(IngressMetricTable).Done(),
c.pipeline[AntreaPolicyIngressRuleTable].BuildFlow(priority100).Cookie(cookiePolicy).
MatchProtocol(binding.ProtocolIP).MatchSrcIP(net.ParseIP("192.168.1.40")).
Expand All @@ -467,17 +467,17 @@ func TestBatchInstallPolicyRuleFlows(t *testing.T) {
MatchProtocol(binding.ProtocolIP).MatchSrcIP(net.ParseIP("192.168.1.51")).
Action().Conjunction(11, 1, 3).Done(),
c.pipeline[AntreaPolicyIngressRuleTable].BuildFlow(priority100).Cookie(cookiePolicy).
MatchReg(int(PortCacheReg), uint32(1)).
MatchRegFieldWithValue(TargetOFPortField, uint32(1)).
Action().Conjunction(10, 2, 2).
Action().Conjunction(11, 2, 3).Done(),
c.pipeline[AntreaPolicyIngressRuleTable].BuildFlow(priority200).Cookie(cookiePolicy).
MatchReg(int(PortCacheReg), uint32(1)).
MatchRegFieldWithValue(TargetOFPortField, uint32(1)).
Action().Conjunction(12, 2, 3).Done(),
c.pipeline[AntreaPolicyIngressRuleTable].BuildFlow(priority100).Cookie(cookiePolicy).
MatchReg(int(PortCacheReg), uint32(2)).
MatchRegFieldWithValue(TargetOFPortField, uint32(2)).
Action().Conjunction(10, 2, 2).Done(),
c.pipeline[AntreaPolicyIngressRuleTable].BuildFlow(priority100).Cookie(cookiePolicy).
MatchReg(int(PortCacheReg), uint32(3)).
MatchRegFieldWithValue(TargetOFPortField, uint32(3)).
Action().Conjunction(11, 2, 3).Done(),
c.pipeline[AntreaPolicyIngressRuleTable].BuildFlow(priority100).Cookie(cookiePolicy).
MatchProtocol(binding.ProtocolTCP).MatchDstPort(8080, nil).
Expand All @@ -492,10 +492,10 @@ func TestBatchInstallPolicyRuleFlows(t *testing.T) {
MatchProtocol(binding.ProtocolIP).MatchCTStateNew(false).MatchCTLabelRange(0, 10, metricIngressRuleIDRange).
Action().GotoTable(conntrackCommitTable).Done(),
c.pipeline[IngressMetricTable].BuildFlow(priorityNormal).Cookie(cookiePolicy).
MatchRegRange(int(marksReg), cnpDenyMark, cnpDenyMarkRange).MatchReg(int(CNPDenyConjIDReg), 11).
MatchRegMark(CnpDenyMark).MatchRegFieldWithValue(CNPDenyConjIDField, 11).
Action().Drop().Done(),
c.pipeline[IngressMetricTable].BuildFlow(priorityNormal).Cookie(cookiePolicy).
MatchRegRange(int(marksReg), cnpDenyMark, cnpDenyMarkRange).MatchReg(int(CNPDenyConjIDReg), 12).
MatchRegMark(CnpDenyMark).MatchRegFieldWithValue(CNPDenyConjIDField, 12).
Action().Drop().Done(),
}
},
Expand Down Expand Up @@ -887,7 +887,7 @@ func newMockRuleFlowBuilder(ctrl *gomock.Controller) *mocks.MockFlowBuilder {
ruleCtAction.EXPECT().CTDone().Return(ruleFlowBuilder).AnyTimes()
ruleAction.EXPECT().CT(true, gomock.Any(), gomock.Any()).Return(ruleCtAction).AnyTimes()
ruleAction.EXPECT().GotoTable(gomock.Any()).Return(ruleFlowBuilder).AnyTimes()
ruleAction.EXPECT().LoadRegRange(gomock.Any(), gomock.Any(), gomock.Any()).Return(ruleFlowBuilder).AnyTimes()
ruleAction.EXPECT().LoadToRegField(gomock.Any(), gomock.Any()).Return(ruleFlowBuilder).AnyTimes()
ruleFlowBuilder.EXPECT().Action().Return(ruleAction).AnyTimes()
ruleFlow = mocks.NewMockFlow(ctrl)
ruleFlowBuilder.EXPECT().Done().Return(ruleFlow).AnyTimes()
Expand All @@ -907,7 +907,7 @@ func newMockMetricFlowBuilder(ctrl *gomock.Controller) *mocks.MockFlowBuilder {
metricFlowBuilder.EXPECT().MatchCTLabelRange(gomock.Any(), gomock.Any(), gomock.Any()).Return(metricFlowBuilder).AnyTimes()
metricAction = mocks.NewMockAction(ctrl)
metricAction.EXPECT().GotoTable(gomock.Any()).Return(metricFlowBuilder).AnyTimes()
metricAction.EXPECT().LoadRegRange(gomock.Any(), gomock.Any(), gomock.Any()).Return(metricFlowBuilder).AnyTimes()
metricAction.EXPECT().LoadToRegField(gomock.Any(), gomock.Any()).Return(metricFlowBuilder).AnyTimes()
metricAction.EXPECT().Drop().Return(metricFlowBuilder).AnyTimes()
metricFlowBuilder.EXPECT().Action().Return(metricAction).AnyTimes()
metricFlow = mocks.NewMockFlow(ctrl)
Expand Down
Loading

0 comments on commit cad46af

Please sign in to comment.