updatre review comments

Signed-off-by: sferna1 <[email protected]>
l3af-project · Oct 4, 2022 · e40337c · e40337c
1 parent a00c81a
commit e40337c
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 39 deletions.
diff --git a/config/config.go b/config/config.go
@@ -95,20 +95,20 @@ func ReadConfig(configPath string) (*Config, error) {
 		PIDFilename:                 LoadConfigString(confReader, "l3afd", "pid-file"),
 		DataCenter:                  LoadConfigString(confReader, "l3afd", "datacenter"),
 		BPFDir:                      LoadConfigString(confReader, "l3afd", "bpf-dir"),
-		BPFLogDir:                   LoadConfigString(confReader, "l3afd", "bpf-log-dir"),
-		MinKernelMajorVer:           LoadConfigInt(confReader, "l3afd", "kernel-major-version"),
-		MinKernelMinorVer:           LoadConfigInt(confReader, "l3afd", "kernel-minor-version"),
+		BPFLogDir:                   LoadOptionalConfigString(confReader, "l3afd", "bpf-log-dir", ""),
+		MinKernelMajorVer:           LoadOptionalConfigInt(confReader, "l3afd", "kernel-major-version", 5),
+		MinKernelMinorVer:           LoadOptionalConfigInt(confReader, "l3afd", "kernel-minor-version", 1),
 		KFRepoURL:                   LoadConfigString(confReader, "kf-repo", "url"),
-		HttpClientTimeout:           LoadConfigDuration(confReader, "l3afd", "http-client-timeout"),
-		MaxNFReStartCount:           LoadConfigInt(confReader, "l3afd", "max-nf-restart-count"),
-		BpfChainingEnabled:          LoadOptionalConfigBool(confReader, "l3afd", "bpf-chaining-enabled", true),
+		HttpClientTimeout:           LoadOptionalConfigDuration(confReader, "l3afd", "http-client-timeout", 10*time.Second),
+		MaxNFReStartCount:           LoadOptionalConfigInt(confReader, "l3afd", "max-nf-restart-count", 3),
+		BpfChainingEnabled:          LoadConfigBool(confReader, "l3afd", "bpf-chaining-enabled"),
 		MetricsAddr:                 LoadConfigString(confReader, "web", "metrics-addr"),
 		KFPollInterval:              LoadOptionalConfigDuration(confReader, "web", "kf-poll-interval", 30*time.Second),
 		NMetricSamples:              LoadOptionalConfigInt(confReader, "web", "n-metric-samples", 20),
-		ShutdownTimeout:             LoadConfigDuration(confReader, "l3afd", "shutdown-timeout"),
+		ShutdownTimeout:             LoadOptionalConfigDuration(confReader, "l3afd", "shutdown-timeout", 5*time.Second),
 		SwaggerApiEnabled:           LoadOptionalConfigBool(confReader, "l3afd", "swagger-api-enabled", false),
 		Environment:                 LoadOptionalConfigString(confReader, "l3afd", "environment", ENV_PROD),
-		BpfMapDefaultPath:           LoadOptionalConfigString(confReader, "l3afd", "BpfMapDefaultPath", "/sys/fs/bpf"),
+		BpfMapDefaultPath:           LoadConfigString(confReader, "l3afd", "BpfMapDefaultPath"),
 		XDPRootProgramName:          LoadOptionalConfigString(confReader, "xdp-root-program", "name", "xdp_root"),
 		XDPRootProgramArtifact:      LoadOptionalConfigString(confReader, "xdp-root-program", "artifact", "l3af_xdp_root.tar.gz"),
 		XDPRootProgramMapName:       LoadOptionalConfigString(confReader, "xdp-root-program", "ingress-map-name", "xdp_root_array"),
@@ -120,13 +120,13 @@ func ReadConfig(configPath string) (*Config, error) {
 		TCRootProgramEgressMapName:  LoadOptionalConfigString(confReader, "tc-root-program", "egress-map-name", "tc/globals/tc_egress_root_array"),
 		TCRootProgramCommand:        LoadOptionalConfigString(confReader, "tc-root-program", "command", "tc_root"),
 		TCRootProgramVersion:        LoadOptionalConfigString(confReader, "tc-root-program", "version", "1.0"),
-		EBPFChainDebugAddr:          LoadOptionalConfigString(confReader, "ebpf-chain-debug", "addr", "0.0.0.0:8899"),
+		EBPFChainDebugAddr:          LoadOptionalConfigString(confReader, "ebpf-chain-debug", "addr", "localhost:8899"),
 		EBPFChainDebugEnabled:       LoadOptionalConfigBool(confReader, "ebpf-chain-debug", "enabled", false),
 		L3afConfigsRestAPIAddr:      LoadOptionalConfigString(confReader, "l3af-configs", "restapi-addr", "localhost:53000"),
-		L3afConfigStoreFileName:     LoadOptionalConfigString(confReader, "l3af-config-store", "filename", "/etc/l3afd/l3af-config.json"),
+		L3afConfigStoreFileName:     LoadConfigString(confReader, "l3af-config-store", "filename"),
 		MTLSEnabled:                 LoadOptionalConfigBool(confReader, "mtls", "enabled", true),
 		MTLSMinVersion:              minTLSVersion,
-		MTLSCertDir:                 LoadOptionalConfigString(confReader, "mtls", "cert-dir", "/etc/l3afd/certs"),
+		MTLSCertDir:                 LoadOptionalConfigString(confReader, "mtls", "cert-dir", ""),
 		MTLSCACertFilename:          LoadOptionalConfigString(confReader, "mtls", "cacert-filename", "ca.pem"),
 		MTLSServerCertFilename:      LoadOptionalConfigString(confReader, "mtls", "server-cert-filename", "server.crt"),
 		MTLSServerKeyFilename:       LoadOptionalConfigString(confReader, "mtls", "server-key-filename", "server.key"),

diff --git a/config/l3afd.cfg b/config/l3afd.cfg
@@ -29,19 +29,17 @@ n-metric-samples: 20
 [xdp-root-program]
 name: xdp-root
 artifact: xdp-root.tar.gz
-ingress-map-name: root_array
+ingress-map-name: xdp_root_array
 command: xdp_root
-version: 1.01
-# user-program-daemon: false
+version: latest
 
 [tc-root-program]
-name: tc_root
+name: tc-root
 artifact: l3af_tc_root.tar.gz
 ingress-map-name: tc/globals/tc_ingress_root_array
 egress-map-name: tc/globals/tc_egress_root_array
 command: tc_root
-version: 1.0
-# user-program-daemon: false
+version: latest
 
 [ebpf-chain-debug]
 addr: 0.0.0.0:8899
@@ -51,7 +49,7 @@ enabled: true
 restapi-addr: localhost:53000
 
 [l3af-config-store]
-filename: "/etc/l3afd/l3af-config.json"
+filename: /var/l3afd/l3af-config.json
 
 [mtls]
 enabled: true

diff --git a/docs/configdoc.md b/docs/configdoc.md
@@ -44,29 +44,33 @@ environment: PROD
 |BpfMapDefaultPath|`"/sys/fs/bpf"`|The base pin path for eBPF maps| Yes |
 
 ## [kf-repo]
-| FieldName     | Example       | Description     | Required        |
-| ------------- | ------------- | --------------- | --------------- |
-|url| `"http://localhost:8000/"`|Default repository from which to download eBPF packages| No |
+| FieldName     | Example       | Description     | Required |
+| ------------- | ------------- | --------------- |----------|
+|url| `"http://localhost:8000/"`|Default repository from which to download eBPF packages| Yes      |
 
 ## [web]
-| FieldName     | Example       | Description     |  Required        |
-| ------------- | ------------- | --------------- |  --------------- |
-|metrics-addr|`"0.0.0.0:8898"`|Prometheus endpoint for pulling/scraping the metrics.  For more info about Prometheus see [prometheus.io](https://prometheus.io/) | Yes |
-|kf-poll-interval|`"30s"`|Periodic interval at which to scrape metrics using Prometheus| Yes |
-|n-metric-samples|`"20"`|Number of Metric Samples| Yes |
+
+| FieldName     | Example       | Description     | Required |
+| ------------- | ------------- | --------------- |----------|
+|metrics-addr|`"0.0.0.0:8898"`|Prometheus endpoint for pulling/scraping the metrics.  For more info about Prometheus see [prometheus.io](https://prometheus.io/) | Yes      |
+|kf-poll-interval|`"30s"`|Periodic interval at which to scrape metrics using Prometheus| No       |
+|n-metric-samples|`"20"`|Number of Metric Samples| No       |
 
 
 ## [xdp-root-program]
+This section is needed when bpf-chaining-enabled is set to true.
+
 | FieldName     | Example       | Description     | Required        |
 | ------------- | ------------- | --------------- | --------------- |
 |name|`"xdp-root"`|Name of subdirectory in which to extract artifact| Yes |
 |artifact|`"xdp-root.tar.gz"`|Filename of xdp-root package. Only tar.gz and .zip formats are supported| Yes |
 |ingress-map-name|`"root_array"`|Ingress map name of xdp-root program| Yes |
 |command|`"xdp_root"`|Command to run xdp-root program| Yes |
 |version|`"1.01"`|Version of xdp-root program| Yes |
-|user-program-daemon|`"false"`|Set to true it requires l3afd to stop the application (via SIGTERM on Linux or SIGKILL on Windows)| Yes |
 
 ## [tc-root-program]
+This section is needed when bpf-chaining-enabled is set to true.
+
 | FieldName     | Example       | Description     | Required        |
 | ------------- | ------------- | --------------- | --------------- |
 |name|`"tc_root"`|Name of subdirectory in which to extract artifact| Yes |
@@ -75,24 +79,23 @@ environment: PROD
 |egress-map-name|`"tc_egress_root_array"`|Egress map name of tc_root program,for more info about ingress/egress check [cilium](https://docs.cilium.io/en/v1.9/concepts/ebpf/intro/)| Yes |
 |command|`"tc_root"`|Command to run tc_root program| Yes |
 |version|`"1.0"`|Version of tc_root program| Yes |
-|user-program-daemon|`"false"`|Set to true it requires l3afd to stop the application (via SIGTERM on Linux or SIGKILL on Windows)| Yes |
 
 ## [l3af-configs]
-| FieldName     | Example       | Description     | Required        |
-| ------------- | ------------- | --------------- | --------------- |
-|restapi-addr|`"localhost:53000"`| Hostname and Port of l3af-configs REST API | Yes |
+| FieldName     | Example       | Description     | Required |
+| ------------- | ------------- | --------------- |----------|
+|restapi-addr|`"localhost:53000"`| Hostname and Port of l3af-configs REST API | No       |
 
 # [l3af-config-store]
 | FieldName     | Example       | Description     | Required        |
 | ------------- | ------------- | --------------- | --------------- |
 |filename|`"/etc/l3afd/l3af-config.json"`|Absolute path of persistent config file where we are storing L3afBPFPrograms objects. For more info see [models](https://github.com/l3af-project/l3afd/blob/main/models/l3afd.go)| Yes |
 
 # [mtls]
-| FieldName     | Example       | Description     | Required        |
-| ------------- | ------------- | --------------- | --------------- |
-|enabled| `"true"` | Boolean to check mtls enabled or not on REST API exposed by l3afd| Yes |
-|min-tls-version|`"1.3"`| Minimum tls version allowed| No |
-|cert-dir|`"/etc/l3af/certs"`|Absolute path of ca certificates. On Linux this points to a filesystem directory, but on Windows it can point to [certificate store](https://docs.microsoft.com/en-us/windows-hardware/drivers/install/certificate-stores) | Yes |
-|server-crt-filename|`"server.crt"`|Server's ca certificate filename| Yes |
-|server-key-filename|`"server.key"`|Server's mtls key filename| Yes |
-|cert-expiry-warning-days|`"30"`|How many days before expiry you want warning| No |
+| FieldName     | Example       | Description     | Required |
+| ------------- | ------------- | --------------- |----------|
+|enabled| `"true"` | Boolean to check mtls enabled or not on REST API exposed by l3afd| No       |
+|min-tls-version|`"1.3"`| Minimum tls version allowed| No       |
+|cert-dir|`"/etc/l3af/certs"`|Absolute path of ca certificates. On Linux this points to a filesystem directory, but on Windows it can point to [certificate store](https://docs.microsoft.com/en-us/windows-hardware/drivers/install/certificate-stores) | No       |
+|server-crt-filename|`"server.crt"`|Server's ca certificate filename| No       |
+|server-key-filename|`"server.key"`|Server's mtls key filename| No       |
+|cert-expiry-warning-days|`"30"`|How many days before expiry you want warning| No       |