Skip to content

Commit 7f9c57d

Browse files
edipascaleedipascale
committed
fix: separate StaticARP enforcer
with the previous implementation, due to the NoReplace flag that is needed on interface objects, static ARPs were not being deleted when the agent configuration for them was removed Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
1 parent d9feaef commit 7f9c57d

File tree

4 files changed

+81
-49
lines changed

4 files changed

+81
-49
lines changed

‎pkg/agent/dozer/bcm/enforcer.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,13 @@ const (
100100
ActionWeightPortChannelConfigFallbackUpdate
101101

102102
ActionWeightInterfaceSubinterfaceIPsDelete
103+
ActionWeghtInterfaceStaticARPDelete
103104
ActionWeightVRFAttachedHostDelete
104105
ActionWeightVRFInterfaceDelete
105106
ActionWeightACLInterfaceDelete
106107
ActionWeightInterfaceSubinterfaceDelete
107108
ActionWeightInterfaceSubinterfaceUpdate
109+
ActionWeghtInterfaceStaticARPUpdate
108110
ActionWeightVRFInterfaceUpdate
109111
ActionWeightVRFAttachedHostUpdate
110112
ActionWeightInterfaceSubinterfaceIPsUpdate

‎pkg/agent/dozer/bcm/plan.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,8 +1157,8 @@ func planExternals(agent *agentapi.Agent, spec *dozer.Spec) error {
11571157
PrefixLen: pointer.To(prefixLen),
11581158
},
11591159
},
1160-
StaticARPs: []dozer.SpecStaticARP{
1161-
{
1160+
StaticARPs: map[string]*dozer.SpecStaticARP{
1161+
virtExtIPStr: {
11621162
IP: virtExtIPStr,
11631163
MAC: attach.L2.MAC,
11641164
},

‎pkg/agent/dozer/bcm/spec_interface.go‎

Lines changed: 70 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ var specInterfaceEnforcer = &DefaultValueEnforcer[string, *dozer.SpecInterface]{
8686
return errors.Wrap(err, "failed to handle interface IPs")
8787
}
8888

89+
actualStaticARPs, desiredStaticARPs := ValueOrNil(actual, desired,
90+
func(value *dozer.SpecInterface) map[string]*dozer.SpecStaticARP { return value.StaticARPs })
91+
if err := specInterfaceVLANStaticARPsEnforcer.Handle(basePath, actualStaticARPs, desiredStaticARPs, actions); err != nil {
92+
return errors.Wrap(err, "failed to handle interface static ARPs")
93+
}
94+
8995
if err := specInterfaceEthernetBaseEnforcer.Handle(basePath, name, actual, desired, actions); err != nil {
9096
return errors.Wrap(err, "failed to handle interface ethernet")
9197
}
@@ -186,28 +192,9 @@ var marshalSpecInterfaceBaseEnforcer = func(name string, value *dozer.SpecInterf
186192
}
187193

188194
if isVLAN(name) {
189-
var neighbors *oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4_Neighbors
190-
if len(value.StaticARPs) > 0 {
191-
neighbors = &oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4_Neighbors{
192-
Neighbor: map[string]*oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4_Neighbors_Neighbor{},
193-
}
194-
for _, sa := range value.StaticARPs {
195-
if sa.IP == "" || sa.MAC == "" {
196-
continue
197-
}
198-
neighbors.Neighbor[sa.IP] = &oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4_Neighbors_Neighbor{
199-
Ip: pointer.To(sa.IP),
200-
Config: &oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4_Neighbors_Neighbor_Config{
201-
Ip: pointer.To(sa.IP),
202-
LinkLayerAddress: pointer.To(sa.MAC),
203-
},
204-
}
205-
}
206-
}
207195
val.RoutedVlan = &oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan{
208196
Ipv4: &oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4{
209-
Config: &oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4_Config{},
210-
Neighbors: neighbors,
197+
Config: &oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4_Config{},
211198
},
212199
}
213200
}
@@ -247,6 +234,56 @@ var specInterfaceVLANIPEnforcer = &DefaultValueEnforcer[string, *dozer.SpecInter
247234
},
248235
}
249236

237+
var specInterfaceVLANStaticARPsEnforcer = &DefaultMapEnforcer[string, *dozer.SpecStaticARP]{
238+
Summary: "Interface %s VLAN Static ARPs",
239+
ValueHandler: specInterfaceVLANStaticARPEnforcer,
240+
}
241+
242+
var specInterfaceVLANStaticARPEnforcer = &DefaultValueEnforcer[string, *dozer.SpecStaticARP]{
243+
Summary: "Interface VLAN Static ARP %s",
244+
Path: "/routed-vlan/ipv4/neighbors/neighbor[ip=%s]",
245+
UpdateWeight: ActionWeghtInterfaceStaticARPUpdate,
246+
DeleteWeight: ActionWeghtInterfaceStaticARPDelete,
247+
Marshal: func(name string, value *dozer.SpecStaticARP) (ygot.ValidatedGoStruct, error) {
248+
return &oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4_Neighbors{
249+
Neighbor: map[string]*oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4_Neighbors_Neighbor{
250+
name: {
251+
Ip: pointer.To(name),
252+
Config: &oc.OpenconfigInterfaces_Interfaces_Interface_RoutedVlan_Ipv4_Neighbors_Neighbor_Config{
253+
Ip: pointer.To(name),
254+
LinkLayerAddress: pointer.To(value.MAC),
255+
},
256+
},
257+
},
258+
}, nil
259+
},
260+
}
261+
262+
var specInterfaceSubinterfaceStaticARPsEnforcer = &DefaultMapEnforcer[string, *dozer.SpecStaticARP]{
263+
Summary: "Subinterface %s Static ARPs",
264+
ValueHandler: specInterfaceSubinterfaceStaticARPEnforcer,
265+
}
266+
267+
var specInterfaceSubinterfaceStaticARPEnforcer = &DefaultValueEnforcer[string, *dozer.SpecStaticARP]{
268+
Summary: "SubInterface Static ARP %s",
269+
Path: "/ipv4/neighbors/neighbor[ip=%s]",
270+
UpdateWeight: ActionWeghtInterfaceStaticARPUpdate,
271+
DeleteWeight: ActionWeghtInterfaceStaticARPDelete,
272+
Marshal: func(name string, value *dozer.SpecStaticARP) (ygot.ValidatedGoStruct, error) {
273+
return &oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface_Ipv4_Neighbors{
274+
Neighbor: map[string]*oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface_Ipv4_Neighbors_Neighbor{
275+
name: {
276+
Ip: pointer.To(name),
277+
Config: &oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface_Ipv4_Neighbors_Neighbor_Config{
278+
Ip: pointer.To(name),
279+
LinkLayerAddress: pointer.To(value.MAC),
280+
},
281+
},
282+
},
283+
}, nil
284+
},
285+
}
286+
250287
var specInterfaceVLANProxyARPEnforcer = &DefaultValueEnforcer[string, *dozer.SpecInterface]{
251288
Summary: "Interface VLAN Proxy-ARP",
252289
Path: "/routed-vlan/ipv4/proxy-arp/config",
@@ -321,6 +358,12 @@ var specInterfaceSubinterfaceEnforcer = &DefaultValueEnforcer[uint32, *dozer.Spe
321358
return errors.Wrap(err, "failed to handle subinterface IPs")
322359
}
323360

361+
actualStaticARPs, desiredStaticARPs := ValueOrNil(actual, desired,
362+
func(value *dozer.SpecSubinterface) map[string]*dozer.SpecStaticARP { return value.StaticARPs })
363+
if err := specInterfaceSubinterfaceStaticARPsEnforcer.Handle(basePath, actualStaticARPs, desiredStaticARPs, actions); err != nil {
364+
return errors.Wrap(err, "failed to handle subinterface static ARPs")
365+
}
366+
324367
return nil // TODO
325368
},
326369
}
@@ -332,7 +375,6 @@ var specInterfaceSubinterfaceBaseEnforcer = &DefaultValueEnforcer[uint32, *dozer
332375
DeleteWeight: ActionWeightInterfaceSubinterfaceDelete,
333376
Marshal: func(idx uint32, value *dozer.SpecSubinterface) (ygot.ValidatedGoStruct, error) {
334377
var vlan *oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface_Vlan
335-
var neighbors *oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface_Ipv4_Neighbors
336378

337379
if value.VLAN != nil {
338380
vlan = &oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface_Vlan{
@@ -342,24 +384,6 @@ var specInterfaceSubinterfaceBaseEnforcer = &DefaultValueEnforcer[uint32, *dozer
342384
}
343385
}
344386

345-
if len(value.StaticARPs) > 0 {
346-
neighbors = &oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface_Ipv4_Neighbors{
347-
Neighbor: map[string]*oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface_Ipv4_Neighbors_Neighbor{},
348-
}
349-
for _, sa := range value.StaticARPs {
350-
if sa.IP == "" || sa.MAC == "" {
351-
continue
352-
}
353-
neighbors.Neighbor[sa.IP] = &oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface_Ipv4_Neighbors_Neighbor{
354-
Ip: pointer.To(sa.IP),
355-
Config: &oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface_Ipv4_Neighbors_Neighbor_Config{
356-
Ip: pointer.To(sa.IP),
357-
LinkLayerAddress: pointer.To(sa.MAC),
358-
},
359-
}
360-
}
361-
}
362-
363387
return &oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces{
364388
Subinterface: map[uint32]*oc.OpenconfigInterfaces_Interfaces_Interface_Subinterfaces_Subinterface{
365389
idx: {
@@ -374,7 +398,6 @@ var specInterfaceSubinterfaceBaseEnforcer = &DefaultValueEnforcer[uint32, *dozer
374398
StaticAnycastGateway: value.AnycastGateways, // TODO extract into a separate code so we can remove values
375399
},
376400
},
377-
Neighbors: neighbors,
378401
},
379402
},
380403
},
@@ -676,6 +699,7 @@ func unmarshalOCInterfaces(agent *agentapi.Agent, ocVal *oc.OpenconfigInterfaces
676699
MTU: mtu,
677700
Subinterfaces: map[uint32]*dozer.SpecSubinterface{},
678701
VLANIPs: map[string]*dozer.SpecInterfaceIP{},
702+
StaticARPs: map[string]*dozer.SpecStaticARP{},
679703
}
680704

681705
// just skip interfaces disabled by Fabric
@@ -690,7 +714,8 @@ func unmarshalOCInterfaces(agent *agentapi.Agent, ocVal *oc.OpenconfigInterfaces
690714
}
691715

692716
subIface := &dozer.SpecSubinterface{
693-
IPs: map[string]*dozer.SpecInterfaceIP{},
717+
IPs: map[string]*dozer.SpecInterfaceIP{},
718+
StaticARPs: map[string]*dozer.SpecStaticARP{},
694719
}
695720

696721
if sub.Ipv4 != nil && sub.Ipv4.Addresses != nil {
@@ -722,10 +747,10 @@ func unmarshalOCInterfaces(agent *agentapi.Agent, ocVal *oc.OpenconfigInterfaces
722747
if n.Config == nil || n.Config.Ip == nil || n.Config.LinkLayerAddress == nil {
723748
continue
724749
}
725-
subIface.StaticARPs = append(subIface.StaticARPs, dozer.SpecStaticARP{
750+
subIface.StaticARPs[*n.Config.Ip] = &dozer.SpecStaticARP{
726751
IP: *n.Config.Ip,
727752
MAC: *n.Config.LinkLayerAddress,
728-
})
753+
}
729754
}
730755
}
731756

@@ -783,10 +808,10 @@ func unmarshalOCInterfaces(agent *agentapi.Agent, ocVal *oc.OpenconfigInterfaces
783808
if n.Config == nil || n.Config.Ip == nil || n.Config.LinkLayerAddress == nil {
784809
continue
785810
}
786-
iface.StaticARPs = append(iface.StaticARPs, dozer.SpecStaticARP{
811+
iface.StaticARPs[*n.Config.Ip] = &dozer.SpecStaticARP{
787812
IP: *n.Config.Ip,
788813
MAC: *n.Config.LinkLayerAddress,
789-
})
814+
}
790815
}
791816
}
792817
}

‎pkg/agent/dozer/dozer.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ type SpecInterface struct {
134134
VLANAnycastGateway []string `json:"vlanAnycastGateway,omitempty"`
135135
Subinterfaces map[uint32]*SpecSubinterface `json:"subinterfaces,omitempty"`
136136
ProxyARP *SpecProxyARP `json:"proxyARP,omitempty"`
137-
StaticARPs []SpecStaticARP `json:"staticARPs,omitempty"`
137+
StaticARPs map[string]*SpecStaticARP `json:"staticARPs,omitempty"`
138138
}
139139

140140
type SpecInterfaceIP struct {
@@ -147,7 +147,7 @@ type SpecSubinterface struct {
147147
IPs map[string]*SpecInterfaceIP `json:"ips,omitempty"`
148148
AnycastGateways []string `json:"anycastGateways,omitempty"`
149149
ProxyARP *SpecProxyARP `json:"proxyARP,omitempty"`
150-
StaticARPs []SpecStaticARP `json:"staticARPs,omitempty"`
150+
StaticARPs map[string]*SpecStaticARP `json:"staticARPs,omitempty"`
151151
}
152152

153153
type SpecMCLAGDomain struct {
@@ -491,6 +491,7 @@ var (
491491
_ SpecPart = (*SpecSubinterface)(nil)
492492
_ SpecPart = (*SpecInterfaceIP)(nil)
493493
_ SpecPart = (*SpecProxyARP)(nil)
494+
_ SpecPart = (*SpecStaticARP)(nil)
494495
_ SpecPart = (*SpecMCLAGDomain)(nil)
495496
_ SpecPart = (*SpecMCLAGInterface)(nil)
496497
_ SpecPart = (*SpecVRF)(nil)
@@ -571,6 +572,10 @@ func (s *SpecProxyARP) IsNil() bool {
571572
return s == nil
572573
}
573574

575+
func (s *SpecStaticARP) IsNil() bool {
576+
return s == nil
577+
}
578+
574579
func (s *SpecMCLAGInterface) IsNil() bool {
575580
return s == nil
576581
}

0 commit comments

Comments
 (0)