Skip to content

Commit 964340a

Browse files
authored
Merge pull request #715 from l1b0k/fix/policy
policy: do not chain cilium when policy is not required
2 parents ade466c + 93c481b commit 964340a

File tree

8 files changed

+157
-96
lines changed

8 files changed

+157
-96
lines changed

cmd/terway-cli/cni.go

Lines changed: 99 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ const (
3838
type feature struct {
3939
EBPF bool
4040
EDT bool
41+
42+
EnableNetworkPolicy bool
4143
}
4244

4345
var (
@@ -56,7 +58,6 @@ func init() {
5658
var cniCmd = &cobra.Command{
5759
Use: "cni",
5860
SilenceUsage: true,
59-
Args: cobra.MinimumNArgs(1),
6061
Run: func(cmd *cobra.Command, args []string) {
6162
err := processCNIConfig(cmd, args)
6263
if err != nil {
@@ -78,12 +79,11 @@ func processCNIConfig(cmd *cobra.Command, args []string) error {
7879
return fmt.Errorf("failed to set feature gates: %v", err)
7980
}
8081

81-
err = processInput(args)
82+
err = processInput()
8283
if err != nil {
8384
return fmt.Errorf("failed process input: %v", err)
8485
}
8586

86-
// mount bpf fs if needed
8787
cni, err := os.ReadFile(outPutPath)
8888
if err != nil {
8989
return err
@@ -92,43 +92,34 @@ func processCNIConfig(cmd *cobra.Command, args []string) error {
9292
if err != nil {
9393
return err
9494
}
95-
for _, plugin := range cniJSON.Path("plugins").Children() {
96-
if plugin.Path("type").Data().(string) == "cilium-cni" {
97-
err = mountHostBpf()
98-
if err != nil {
99-
return err
100-
}
101-
}
102-
}
10395

10496
return storeRuntimeConfig(nodeCapabilitiesFile, cniJSON)
10597
}
10698

107-
func processInput(files []string) error {
99+
func processInput() error {
100+
cm, err := getAllConfig(eniConfBasePath)
101+
if err != nil {
102+
return err
103+
}
104+
105+
input := cm.cniConfig
106+
if cm.cniConfigList != nil {
107+
input = cm.cniConfigList
108+
}
109+
108110
var configs [][]byte
109-
for _, file := range files {
110-
out, err := os.ReadFile(file)
111-
if err != nil {
112-
if os.IsNotExist(err) {
113-
continue
114-
}
115-
return err
116-
}
117-
c, err := gabs.ParseJSON(out)
118-
if err != nil {
119-
return err
120-
}
121-
if c.Exists("plugins") {
122-
for _, cc := range c.Path("plugins").Children() {
123-
configs = append(configs, cc.Bytes())
124-
}
125-
} else {
126-
configs = append(configs, out)
111+
c, err := gabs.ParseJSON(input)
112+
if err != nil {
113+
return err
114+
}
115+
if c.Exists("plugins") {
116+
for _, cc := range c.Path("plugins").Children() {
117+
configs = append(configs, cc.Bytes())
127118
}
128-
break
119+
} else {
120+
configs = append(configs, input)
129121
}
130122

131-
var err error
132123
f := feature{}
133124
f.EBPF = _checkKernelVersion(4, 19, 0)
134125

@@ -138,6 +129,9 @@ func processInput(files []string) error {
138129
return err
139130
}
140131
}
132+
133+
f.EnableNetworkPolicy = cm.enableNetworkPolicy
134+
141135
out, err := mergeConfigList(configs, &f)
142136
if err != nil {
143137
return err
@@ -211,65 +205,62 @@ func mergeConfigList(configs [][]byte, f *feature) (string, error) {
211205
return "", fmt.Errorf("network_policy_provider type error")
212206
}
213207
}
214-
if plugin.Exists("eniip_virtual_type") {
215-
virtualType, ok := plugin.Path("eniip_virtual_type").Data().(string)
216-
if !ok {
217-
return "", fmt.Errorf("eniip_virtual_type not found")
218-
}
219-
if !ebpfSupport {
220-
err = plugin.Delete("eniip_virtual_type")
221-
if err != nil {
222-
return "", err
223-
}
224-
} else {
225-
requireIPvlan := false
226208

227-
switch strings.ToLower(virtualType) {
228-
case dataPathVeth, dataPathDefault:
229-
datapath = dataPathVeth
209+
virtualType, ok := plugin.Path("eniip_virtual_type").Data().(string)
210+
if !ok {
211+
virtualType = dataPathVeth
212+
}
213+
if !ebpfSupport {
214+
_ = plugin.Delete("eniip_virtual_type")
215+
} else {
216+
requireIPvlan := false
217+
218+
switch strings.ToLower(virtualType) {
219+
case dataPathVeth, dataPathDefault:
220+
datapath = dataPathVeth
230221

231-
// only for terway-eniip
232-
if ebpfSupport && networkPolicyProvider == NetworkPolicyProviderEBPF {
233-
requireEBPFChainer = true
222+
// only for terway-eniip
223+
if ebpfSupport && networkPolicyProvider == NetworkPolicyProviderEBPF {
224+
allow, err := allowEBPFNetworkPolicy(f.EnableNetworkPolicy)
225+
if err != nil {
226+
return "", err
234227
}
235-
case dataPathIPvlan:
236-
requireIPvlan = true
237-
datapath = dataPathIPvlan
238-
239-
fallthrough
240-
case dataPathV2:
241-
requireEBPFChainer = true
242-
243-
if requireIPvlan && !_switchDataPathV2() {
244-
fmt.Printf("keep ipvlan mode %v %v\n", requireIPvlan, !_switchDataPathV2())
245-
_, err = plugin.Set(dataPathIPvlan, "eniip_virtual_type")
246-
if err != nil {
247-
return "", err
248-
}
249-
} else {
250-
fmt.Printf("datapathv2 enabled\n")
251-
_, err = plugin.Set(dataPathV2, "eniip_virtual_type")
252-
if err != nil {
253-
return "", err
254-
}
255-
256-
datapath = dataPathV2
228+
if allow {
229+
requireEBPFChainer = true
257230
}
231+
}
232+
case dataPathIPvlan:
233+
requireIPvlan = true
234+
datapath = dataPathIPvlan
235+
236+
fallthrough
237+
case dataPathV2:
238+
requireEBPFChainer = true
258239

259-
if edtSupport {
260-
_, err = plugin.Set("edt", "bandwidth_mode")
261-
} else {
262-
_, err = plugin.Set("tc", "bandwidth_mode")
240+
if requireIPvlan && !_switchDataPathV2() {
241+
fmt.Printf("keep ipvlan mode %v %v\n", requireIPvlan, !_switchDataPathV2())
242+
_, err = plugin.Set(dataPathIPvlan, "eniip_virtual_type")
243+
if err != nil {
244+
return "", err
263245
}
246+
} else {
247+
fmt.Printf("datapathv2 enabled\n")
248+
_, err = plugin.Set(dataPathV2, "eniip_virtual_type")
264249
if err != nil {
265250
return "", err
266251
}
252+
253+
datapath = dataPathV2
254+
}
255+
256+
if edtSupport {
257+
_, err = plugin.Set("edt", "bandwidth_mode")
258+
} else {
259+
_, err = plugin.Set("tc", "bandwidth_mode")
260+
}
261+
if err != nil {
262+
return "", err
267263
}
268-
}
269-
} else {
270-
datapath = dataPathVeth
271-
if ebpfSupport && networkPolicyProvider == NetworkPolicyProviderEBPF {
272-
requireEBPFChainer = true
273264
}
274265
}
275266
}
@@ -315,20 +306,38 @@ func storeRuntimeConfig(filePath string, container *gabs.Container) error {
315306
return err
316307
}
317308

309+
hasCilium := false
318310
// write back current runtime config
319311
for _, plugin := range container.Path("plugins").Children() {
320-
if plugin.Path("type").Data().(string) != pluginTypeTerway {
321-
continue
322-
}
323-
if plugin.Exists("network_policy_provider") {
324-
networkPolicyProvider := plugin.Path("network_policy_provider").Data().(string)
325-
store.Set(nodecap.NodeCapabilityNetworkPolicyProvider, networkPolicyProvider)
312+
pluginType, ok := plugin.Path("type").Data().(string)
313+
if !ok {
314+
return fmt.Errorf("type must be string")
326315
}
327-
if plugin.Exists("eniip_virtual_type") {
328-
datapath := plugin.Path("eniip_virtual_type").Data().(string)
329-
store.Set(nodecap.NodeCapabilityDataPath, datapath)
316+
switch pluginType {
317+
case pluginTypeCilium:
318+
// mount bpf fs if needed
319+
320+
err = mountHostBpf()
321+
if err != nil {
322+
return err
323+
}
324+
hasCilium = true
325+
case pluginTypeTerway:
326+
if plugin.Exists("network_policy_provider") {
327+
networkPolicyProvider := plugin.Path("network_policy_provider").Data().(string)
328+
store.Set(nodecap.NodeCapabilityNetworkPolicyProvider, networkPolicyProvider)
329+
}
330+
if plugin.Exists("eniip_virtual_type") {
331+
datapath := plugin.Path("eniip_virtual_type").Data().(string)
332+
store.Set(nodecap.NodeCapabilityDataPath, datapath)
333+
}
330334
}
331335
}
336+
if hasCilium {
337+
store.Set(nodecap.NodeCapabilityHasCiliumChainer, True)
338+
} else {
339+
store.Set(nodecap.NodeCapabilityHasCiliumChainer, False)
340+
}
332341

333342
return store.Save()
334343
}

cmd/terway-cli/cni_linux.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,39 @@ func switchDataPathV2() bool {
2727
return errors.As(err, &netlink.LinkNotFoundError{})
2828
}
2929

30+
// allowEBPFNetworkPolicy check in veth datapath
31+
// policy
32+
// false -> true:
33+
// old node(has cilium already) keep old behave
34+
// old node(do not has cilium) keep old behave
35+
// new node ( based on user require).
36+
// true -> false: keep cilium chain, but disable policy
37+
func allowEBPFNetworkPolicy(require bool) (bool, error) {
38+
store := nodecap.NewFileNodeCapabilities(nodeCapabilitiesFile)
39+
if err := store.Load(); err != nil {
40+
return false, err
41+
}
42+
switch store.Get(nodecap.NodeCapabilityHasCiliumChainer) {
43+
case True:
44+
fmt.Printf("has prev cilium chainer\n")
45+
return true, nil
46+
case False:
47+
fmt.Printf("no prev cilium chainer\n")
48+
return false, nil
49+
}
50+
51+
_, err := netlink.LinkByName("cilium_net")
52+
if err == nil {
53+
fmt.Printf("link cilium_net exist\n")
54+
return true, nil
55+
}
56+
if !errors.As(err, &netlink.LinkNotFoundError{}) {
57+
return false, err
58+
}
59+
60+
return require, nil
61+
}
62+
3063
func checkKernelVersion(k, major, minor int) bool {
3164
return kernel.CheckKernelVersion(k, major, minor)
3265
}

cmd/terway-cli/cni_unsupport.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,7 @@ func switchDataPathV2() bool {
99
func checkKernelVersion(k, major, minor int) bool {
1010
return false
1111
}
12+
13+
func allowEBPFNetworkPolicy(enable bool) (bool, error) {
14+
return enable, nil
15+
}

cmd/terway-cli/common.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ const (
1111
)
1212
const eniConfBasePath = "/etc/eni"
1313

14+
const (
15+
True = "true"
16+
False = "false"
17+
)
18+
1419
type TerwayConfig struct {
1520
enableNetworkPolicy bool
1621
enableInClusterLB bool
@@ -71,7 +76,7 @@ func getAllConfig(base string) (*TerwayConfig, error) {
7176
return nil, err
7277
}
7378
}
74-
if string(r) == "true" {
79+
if string(r) == True {
7580
cfg.enableInClusterLB = true
7681
}
7782

cmd/terway-cli/node.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,9 @@ func dualStack(cmd *cobra.Command, args []string) error {
166166
val := ""
167167
switch eniCfg.IPStack {
168168
case "dual", "ipv6":
169-
val = "true"
169+
val = True
170170
default:
171-
val = "false"
171+
val = False
172172
}
173173

174174
err := store.Load()

cmd/terway-cli/policy.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ type PolicyConfig struct {
2424
HealthCheckPort string
2525
IPv6 bool
2626
InClusterLoadBalance bool
27+
HasCiliumChainer bool
2728
}
2829

2930
type CNIConfig struct {
@@ -61,7 +62,7 @@ func getPolicyConfig(capFilePath string) (*PolicyConfig, error) {
6162
return nil, err
6263
}
6364

64-
if store.Get(nodecap.NodeCapabilityIPv6) == ("true") {
65+
if store.Get(nodecap.NodeCapabilityIPv6) == True {
6566
cfg.IPv6 = true
6667
}
6768

@@ -70,6 +71,7 @@ func getPolicyConfig(capFilePath string) (*PolicyConfig, error) {
7071
}
7172
cfg.Datapath = store.Get(nodecap.NodeCapabilityDataPath)
7273
cfg.PolicyProvider = store.Get(nodecap.NodeCapabilityNetworkPolicyProvider)
74+
cfg.HasCiliumChainer = store.Get(nodecap.NodeCapabilityHasCiliumChainer) == True
7375

7476
cfg.HealthCheckPort = os.Getenv("FELIX_HEALTHPORT")
7577
if cfg.HealthCheckPort == "" {
@@ -107,6 +109,9 @@ func initPolicy(cmd *cobra.Command, args []string) error {
107109
}
108110
return runSocat(cfg)
109111
}
112+
if !cfg.HasCiliumChainer {
113+
return runSocat(cfg)
114+
}
110115
fmt.Printf("enable ebpf provider, run cilium")
111116
fallthrough
112117
case dataPathIPvlan, dataPathV2:
@@ -167,6 +172,10 @@ func runCalico(cfg *PolicyConfig) error {
167172
}
168173

169174
func runCilium(cfg *PolicyConfig) error {
175+
if !cfg.HasCiliumChainer {
176+
return fmt.Errorf("no cilium chainer is installed")
177+
}
178+
170179
extraArgs, err := parsePolicyConfig()
171180
if err != nil {
172181
return err
@@ -193,7 +202,6 @@ func runCilium(cfg *PolicyConfig) error {
193202
"--enable-l7-proxy=false",
194203
"--ipam=cluster-pool",
195204
"--enable-runtime-device-detection=true",
196-
"--enable-policy=" + fmt.Sprintf("%t", cfg.EnableNetworkPolicy),
197205
"--agent-health-port=" + cfg.HealthCheckPort,
198206
}
199207
if cfg.EnableNetworkPolicy {

0 commit comments

Comments
 (0)