Skip to content

Commit

Permalink
Merge pull request #694 from Nordix/fix-pbr
Browse files Browse the repository at this point in the history 
Fix Missing Policy Routes
  • Loading branch information
@denis-tingaikin
denis-tingaikin authored Oct 28, 2024
2 parents 3e29bf1 + 2b15935 commit 6b5aa1e
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 27 deletions.
48 changes: 25 additions & 23 deletions pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// Copyright (c) 2023 Cisco and/or its affiliates.
//
// Copyright (c) 2023 Nordix Foundation.
// Copyright (c) 2023-2024 Nordix Foundation.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down Expand Up @@ -56,7 +56,7 @@ func create(ctx context.Context, conn *networkservice.Connection, tableIDs *gene
ifName := mechanism.GetInterfaceName()
l, err := netlinkHandle.LinkByName(ifName)
if err != nil {
return errors.Wrapf(err, "failed to find link %s", ifName)
return errors.Wrapf(err, "iprule: failed to create policy rules for interface %s", ifName)
}
connID := conn.GetId()
ps, ok := tableIDs.Load(connID)
Expand All @@ -71,7 +71,7 @@ func create(ctx context.Context, conn *networkservice.Connection, tableIDs *gene
// Get netns for key to namespace to routing tableID map
netNS, err := nshandle.FromURL(mechanism.GetNetNSURL())
if err != nil {
return err
return errors.Wrapf(err, "iprule: failed to create policy rules in namespace: %s", mechanism.GetNetNSURL())
}

// Get policies to add and to remove
Expand Down Expand Up @@ -100,7 +100,7 @@ func create(ctx context.Context, conn *networkservice.Connection, tableIDs *gene
log.FromContext(ctx).
WithField("nsrtid", nsrtid).
WithField("ConnID", storedConnID).
Debug("storedTableID")
Debug("iprule:createNetnsRTableNextID")
if connID == storedConnID {
// No other connection adding policy using this free routing table ID
break
Expand Down Expand Up @@ -187,7 +187,7 @@ func policyToRule(policy *networkservice.PolicyRoute) (*netlink.Rule, error) {
}
srcPortRange, err := networkservice.ParsePortRange(policy.SrcPort)
if err != nil {
return nil, errors.Wrapf(err, "failed to parse port range %s", policy.DstPort)
return nil, errors.Wrapf(err, "failed to parse port range %s", policy.SrcPort)
}
if srcPortRange != nil {
rule.Sport = netlink.NewRulePortRange(srcPortRange.Start, srcPortRange.End)
Expand All @@ -212,7 +212,7 @@ func ruleAdd(ctx context.Context, handle *netlink.Handle, policy *networkservice
WithField("Table", tableID).
WithField("duration", time.Since(now)).
WithField("netlink", "RuleAdd").Errorf("error %+v", err)
return errors.Wrap(err, "failed to add rule")
return errors.Wrap(err, "iprule: failed to add rule")
}
log.FromContext(ctx).
WithField("From", policy.From).
Expand All @@ -233,7 +233,7 @@ func defaultRoute() *networkservice.Route {

func routeAdd(ctx context.Context, handle *netlink.Handle, l netlink.Link, route *networkservice.Route, tableID int) error {
if route.GetPrefixIPNet() == nil {
return errors.New("kernelRoute prefix must not be nil")
return errors.New("iprule: kernelRoute prefix must not be nil")
}
dst := route.GetPrefixIPNet()
dst.IP = dst.IP.Mask(dst.Mask)
Expand Down Expand Up @@ -261,7 +261,7 @@ func routeAdd(ctx context.Context, handle *netlink.Handle, l netlink.Link, route
WithField("Table", tableID).
WithField("duration", time.Since(now)).
WithField("netlink", "RouteReplace").Errorf("error %+v", err)
return errors.Wrap(err, "failed to add route")
return errors.Wrap(err, "iprule: failed to add route")
}
log.FromContext(ctx).
WithField("link.Name", l.Attrs().Name).
Expand All @@ -285,13 +285,13 @@ func del(ctx context.Context, conn *networkservice.Connection, tableIDs *generic
ifName := mechanism.GetInterfaceName()
l, err := netlinkHandle.LinkByName(ifName)
if err != nil {
return errors.Wrapf(err, "failed to find link %s", ifName)
return errors.Wrapf(err, "iprule: failed to delete policy rules for interface %s", ifName)
}
ps, ok := tableIDs.LoadAndDelete(conn.GetId())
if ok {
netNS, err := nshandle.FromURL(mechanism.GetNetNSURL())
if err != nil {
return err
return errors.Wrapf(err, "iprule: failed to delete policy rules in namespace: %s", mechanism.GetNetNSURL())
}
for tableID, policy := range ps {
if err := delRule(ctx, netlinkHandle, policy, tableID, l.Attrs().Index, createNetnsRTableNextID(netNS.UniqueId(), tableID), nsRTableNextIDToConnID); err != nil {
Expand All @@ -317,7 +317,7 @@ func delRuleOnly(ctx context.Context, handle *netlink.Handle, policy *networkser
WithField("SrcPort", policy.SrcPort).
WithField("duration", time.Since(now)).
WithField("netlink", "RuleDel").Errorf("error %+v", err)
return errors.Wrapf(err, "failed to delete rule")
return errors.Wrapf(err, "iprule: failed to delete rule")
}
log.FromContext(ctx).
WithField("From", policy.From).
Expand All @@ -329,14 +329,16 @@ func delRuleOnly(ctx context.Context, handle *netlink.Handle, policy *networkser
return nil
}

func delRule(ctx context.Context, handle *netlink.Handle, policy *networkservice.PolicyRoute, tableID, linkIndex int, nsRTableKey netnsRTableNextID, nsRTableNextIDToConnID *genericsync.Map[netnsRTableNextID, string]) error {
if err := flushTable(ctx, handle, tableID, linkIndex); err != nil {
return err
func delRule(ctx context.Context, handle *netlink.Handle, policy *networkservice.PolicyRoute, tableID, linkIndex int, nsRTableKey netnsRTableNextID, nsRTableNextIDToConnID *genericsync.Map[netnsRTableNextID, string]) (err error) {
if err = flushTable(ctx, handle, tableID, linkIndex); err == nil {
nsRTableNextIDToConnID.Delete(nsRTableKey)
}
nsRTableNextIDToConnID.Delete(nsRTableKey)

return delRuleOnly(ctx, handle, policy)
if errDelRule := delRuleOnly(ctx, handle, policy); errDelRule != nil {
return errDelRule
}
return err
}

func flushTable(ctx context.Context, handle *netlink.Handle, tableID, linkIndex int) error {
routes, err := handle.RouteListFiltered(netlink.FAMILY_ALL,
&netlink.Route{
Expand All @@ -345,7 +347,7 @@ func flushTable(ctx context.Context, handle *netlink.Handle, tableID, linkIndex
},
netlink.RT_FILTER_TABLE)
if err != nil {
return errors.Wrapf(err, "failed to list routes")
return errors.Wrapf(err, "iprule: failed to flush routing for tableID:%d, linkID %d", tableID, linkIndex)
}
for i := 0; i < len(routes); i++ {
// This conditions means the default route. We should delete it properly
Expand All @@ -358,7 +360,7 @@ func flushTable(ctx context.Context, handle *netlink.Handle, tableID, linkIndex
}
err := handle.RouteDel(&routes[i])
if err != nil {
return errors.Wrapf(err, "failed to delete route: %v", routes[i].String())
return errors.Wrapf(err, "iprule: failed to delete route: %v", routes[i].String())
}

log.FromContext(ctx).
Expand All @@ -367,7 +369,7 @@ func flushTable(ctx context.Context, handle *netlink.Handle, tableID, linkIndex
}
log.FromContext(ctx).
WithField("tableID", tableID).
WithField("netlink", "flushTable").Debug("completed")
WithField("iprule", "flushTable").Debug("completed")
return nil
}

Expand All @@ -378,12 +380,12 @@ func getFreeTableID(ctx context.Context, handle *netlink.Handle) (int, error) {
},
netlink.RT_FILTER_TABLE)
if err != nil {
return 0, errors.Wrapf(err, "getFreeTableID: failed to list routes")
return 0, errors.Wrapf(err, "iprule: failed to get free routing table ID, no routes")
}

rules, err := handle.RuleList(netlink.FAMILY_ALL)
if err != nil {
return 0, errors.Wrapf(err, "getFreeTableID: failed to list rules")
return 0, errors.Wrapf(err, "iprule: failed to get free routing table ID, no rules")
}

// tableID = 0 is reserved
Expand All @@ -405,7 +407,7 @@ func getFreeTableID(ctx context.Context, handle *netlink.Handle) (int, error) {
}
log.FromContext(ctx).
WithField("tableID", tableID).
WithField("netlink", "getFreeTableID").Debug("completed")
WithField("iprule", "getFreeTableID").Debug("completed")

return tableID, nil
}
8 changes: 4 additions & 4 deletions pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/heal.go
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Copyright (c) 2022 Doc.ai and/or its affiliates.
//
// Copyright (c) 2021-2022 Nordix Foundation.
//
// Copyright (c) 2023 Cisco and/or its affiliates.
//
// Copyright (c) 2021-2024 Nordix Foundation.
//
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -57,12 +57,12 @@ func recoverTableIDs(ctx context.Context, conn *networkservice.Connection, table
ifName := mechanism.GetInterfaceName()
l, err := netlinkHandle.LinkByName(ifName)
if err != nil {
return errors.Wrapf(err, "failed to find link %s", ifName)
return errors.Wrapf(err, "iprule: failed to recover table IDs for interface: %s", ifName)
}

podRules, err := netlinkHandle.RuleList(netlink.FAMILY_ALL)
if err != nil {
return errors.Wrap(err, "failed to get list of rules")
return errors.Wrapf(err, "iprule: failed to recover table IDs in namespace: %s", mechanism.GetNetNSURL())
}

tableIDtoPolicyMap := make(map[int]*networkservice.PolicyRoute)
Expand Down

0 comments on commit 6b5aa1e

Please sign in to comment.