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.

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

endpoint: Remove the duplicate ENABLE_SIP_VERIFICATION in ep_config.h #25533

Merged
merged 3 commits into from
Jun 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions pkg/datapath/linux/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -985,10 +985,6 @@ func (h *HeaderfileWriter) writeTemplateConfig(fw *bufio.Writer, e datapath.Endp
fmt.Fprintf(fw, "#define ENABLE_ROUTING 1\n")
}

if !e.DisableSIPVerification() {
Copy link
Contributor

Choose a reason for hiding this comment

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

With this, I think we can just remove this from the EndpointConfiguration interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good, let me try to fix that

fmt.Fprintf(fw, "#define ENABLE_SIP_VERIFICATION 1\n")
}

if !option.Config.EnableHostLegacyRouting && option.Config.DirectRoutingDevice != "" {
directRoutingIface := option.Config.DirectRoutingDevice
directRoutingIfIndex, err := link.GetIfIndex(directRoutingIface)
Expand Down
4 changes: 0 additions & 4 deletions pkg/datapath/types/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,6 @@ type CompileTimeConfiguration interface {

// IsHost returns true if the endpoint is the host endpoint.
IsHost() bool

// DisableSIPVerification returns true if the endpoint wishes to skip
// source IP verification
DisableSIPVerification() bool
}

// EndpointConfiguration provides datapath implementations a clean interface
Expand Down
5 changes: 5 additions & 0 deletions pkg/endpoint/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ func NewEndpointFromChangeModel(ctx context.Context, owner regeneration.Owner, p

if base.DatapathConfiguration != nil {
ep.DatapathConfiguration = *base.DatapathConfiguration
// We need to make sure DatapathConfiguration.DisableSipVerification value
// overrides the value of SourceIPVerification runtime option of the endpoint.
if ep.DatapathConfiguration.DisableSipVerification {
ep.updateAndOverrideEndpointOptions(option.OptionMap{option.SourceIPVerification: option.OptionDisabled})
}
}

if base.Labels != nil {
Expand Down
6 changes: 0 additions & 6 deletions pkg/endpoint/bpf.go
Original file line number Diff line number Diff line change
Expand Up @@ -1455,12 +1455,6 @@ func (e *Endpoint) RequireEndpointRoute() bool {
return e.DatapathConfiguration.InstallEndpointRoute
}

// DisableSIPVerification returns true if the endpoint wants to skip
// srcIP verification
func (e *Endpoint) DisableSIPVerification() bool {
return e.DatapathConfiguration.DisableSipVerification
}

// GetPolicyVerdictLogFilter returns the PolicyVerdictLogFilter that would control
// the creation of policy verdict logs. Value of VerdictLogFilter needs to be
// consistent with how it is used in policy_verdict_filter_allow() in bpf/lib/policy_log.h
Expand Down
8 changes: 0 additions & 8 deletions pkg/endpoint/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ type epInfoCache struct {
requireEgressProg bool
requireRouting bool
requireEndpointRoute bool
disableSIPVerification bool
policyVerdictLogFilter uint32
cidr4PrefixLengths, cidr6PrefixLengths []int
options *option.IntOptions
Expand Down Expand Up @@ -73,7 +72,6 @@ func (e *Endpoint) createEpInfoCache(epdir string) *epInfoCache {
requireEgressProg: e.RequireEgressProg(),
requireRouting: e.RequireRouting(),
requireEndpointRoute: e.RequireEndpointRoute(),
disableSIPVerification: e.DisableSIPVerification(),
policyVerdictLogFilter: e.GetPolicyVerdictLogFilter(),
cidr4PrefixLengths: cidr4,
cidr6PrefixLengths: cidr6,
Expand Down Expand Up @@ -174,12 +172,6 @@ func (ep *epInfoCache) RequireEndpointRoute() bool {
return ep.requireEndpointRoute
}

// DisableSIPVerification returns true if the endpoint wants to skip
// srcIP verification
func (ep *epInfoCache) DisableSIPVerification() bool {
return ep.disableSIPVerification
}

func (ep *epInfoCache) GetPolicyVerdictLogFilter() uint32 {
return ep.policyVerdictLogFilter
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/endpoint/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

. "github.com/cilium/checkmate"

"github.com/cilium/cilium/api/v1/models"
"github.com/cilium/cilium/pkg/datapath/fake"
datapath "github.com/cilium/cilium/pkg/datapath/types"
"github.com/cilium/cilium/pkg/endpoint/regeneration"
Expand Down Expand Up @@ -255,6 +256,16 @@ func (s *EndpointSuite) TestEndpointStatus(c *C) {
c.Assert(eps.String(), Equals, "OK")
}

func (s *EndpointSuite) TestEndpointDatapathOptions(c *C) {
e, err := NewEndpointFromChangeModel(context.TODO(), s, s, testipcache.NewMockIPCache(), &FakeEndpointProxy{}, s.mgr, &models.EndpointChangeRequest{
DatapathConfiguration: &models.EndpointDatapathConfiguration{
DisableSipVerification: true,
},
})
c.Assert(err, IsNil)
c.Assert(e.Options.GetValue(option.SourceIPVerification), Equals, option.OptionDisabled)
}

func (s *EndpointSuite) TestEndpointUpdateLabels(c *C) {
e := NewEndpointWithState(s, s, testipcache.NewMockIPCache(), &FakeEndpointProxy{}, testidentity.NewMockIdentityAllocator(nil), 100, StateWaitingForIdentity)

Expand Down
2 changes: 0 additions & 2 deletions pkg/endpointmanager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,6 @@ func (mgr *endpointManager) RestoreEndpoint(ep *endpoint.Endpoint) error {

// AddEndpoint takes the prepared endpoint object and starts managing it.
func (mgr *endpointManager) AddEndpoint(owner regeneration.Owner, ep *endpoint.Endpoint, reason string) (err error) {
ep.SetDefaultConfiguration(false)

if ep.ID != 0 {
return fmt.Errorf("Endpoint ID is already set to %d", ep.ID)
}
Expand Down
1 change: 0 additions & 1 deletion pkg/testutils/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ func (e *TestEndpoint) RequireARPPassthrough() bool { return fal
func (e *TestEndpoint) RequireEgressProg() bool { return false }
func (e *TestEndpoint) RequireRouting() bool { return false }
func (e *TestEndpoint) RequireEndpointRoute() bool { return false }
func (e *TestEndpoint) DisableSIPVerification() bool { return false }
func (e *TestEndpoint) GetPolicyVerdictLogFilter() uint32 { return 0xffff }
func (e *TestEndpoint) GetCIDRPrefixLengths() ([]int, []int) { return nil, nil }
func (e *TestEndpoint) GetID() uint64 { return e.Id }
Expand Down