From 646b1f5928e01392b8152db8c4d71adf7350b3e4 Mon Sep 17 00:00:00 2001 From: ganglv <88995770+ganglyu@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:27:03 +0800 Subject: [PATCH 1/4] Increase dbus timeout for gcu (#295) Why I did it GNMI test failed on some sonic devices, root cause is gcu performance. How I did it Increase dbus timeout. How to verify it Run end to end test. --- sonic_service_client/dbus_client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sonic_service_client/dbus_client.go b/sonic_service_client/dbus_client.go index 79835436..73af426c 100644 --- a/sonic_service_client/dbus_client.go +++ b/sonic_service_client/dbus_client.go @@ -124,7 +124,7 @@ func (c *DbusClient) ApplyPatchYang(patch string) error { busName := c.busNamePrefix + modName busPath := c.busPathPrefix + modName intName := c.intNamePrefix + modName + ".apply_patch_yang" - _, err := DbusApi(busName, busPath, intName, 180, patch) + _, err := DbusApi(busName, busPath, intName, 240, patch) return err } @@ -134,7 +134,7 @@ func (c *DbusClient) ApplyPatchDb(patch string) error { busName := c.busNamePrefix + modName busPath := c.busPathPrefix + modName intName := c.intNamePrefix + modName + ".apply_patch_db" - _, err := DbusApi(busName, busPath, intName, 180, patch) + _, err := DbusApi(busName, busPath, intName, 240, patch) return err } @@ -190,4 +190,4 @@ func (c *DbusClient) GetFileStat(path string) (map[string]string, error) { } data, _ := result.(map[string]string) return data, nil -} \ No newline at end of file +} From 2b7f8a1b712cfc83b28d434eede9a3be30532c78 Mon Sep 17 00:00:00 2001 From: Hua Liu <58683130+liuh-80@users.noreply.github.com> Date: Sat, 12 Oct 2024 10:06:55 +0800 Subject: [PATCH 2/4] Add mgmt VRF support. (#290) * Add mgmt VRF support * Update client_test.go --- gnmi_server/client_subscribe.go | 2 +- gnmi_server/server.go | 7 ++++--- sonic_data_client/client_test.go | 6 +++--- sonic_data_client/mixed_db_client.go | 14 +++++++------- telemetry/telemetry.go | 3 +++ 5 files changed, 18 insertions(+), 14 deletions(-) diff --git a/gnmi_server/client_subscribe.go b/gnmi_server/client_subscribe.go index 8c36642d..bf791e79 100644 --- a/gnmi_server/client_subscribe.go +++ b/gnmi_server/client_subscribe.go @@ -161,7 +161,7 @@ func (c *Client) Run(stream gnmipb.GNMI_SubscribeServer) (err error) { if origin == "openconfig" { dc, err = sdc.NewTranslClient(prefix, paths, ctx, extensions, sdc.TranslWildcardOption{}) } else if IsNativeOrigin(origin) { - dc, err = sdc.NewMixedDbClient(paths, prefix, origin, gnmipb.Encoding_JSON_IETF, "") + dc, err = sdc.NewMixedDbClient(paths, prefix, origin, gnmipb.Encoding_JSON_IETF, "", "") } else if len(origin) != 0 { return grpc.Errorf(codes.Unimplemented, "Unsupported origin: %s", origin) } else if target == "" { diff --git a/gnmi_server/server.go b/gnmi_server/server.go index be490f54..f3ec24ce 100644 --- a/gnmi_server/server.go +++ b/gnmi_server/server.go @@ -85,6 +85,7 @@ type Config struct { ZmqPort string IdleConnDuration int ConfigTableName string + Vrf string } var AuthLock sync.Mutex @@ -409,7 +410,7 @@ func (s *Server) Get(ctx context.Context, req *gnmipb.GetRequest) (*gnmipb.GetRe } } if check := IsNativeOrigin(origin); check { - dc, err = sdc.NewMixedDbClient(paths, prefix, origin, encoding, s.config.ZmqPort) + dc, err = sdc.NewMixedDbClient(paths, prefix, origin, encoding, s.config.ZmqPort, s.config.Vrf) } else { dc, err = sdc.NewTranslClient(prefix, paths, ctx, extensions) } @@ -507,7 +508,7 @@ func (s *Server) Set(ctx context.Context, req *gnmipb.SetRequest) (*gnmipb.SetRe common_utils.IncCounter(common_utils.GNMI_SET_FAIL) return nil, grpc.Errorf(codes.Unimplemented, "GNMI native write is disabled") } - dc, err = sdc.NewMixedDbClient(paths, prefix, origin, encoding, s.config.ZmqPort) + dc, err = sdc.NewMixedDbClient(paths, prefix, origin, encoding, s.config.ZmqPort, s.config.Vrf) } else { if s.config.EnableTranslibWrite == false { common_utils.IncCounter(common_utils.GNMI_SET_FAIL) @@ -584,7 +585,7 @@ func (s *Server) Capabilities(ctx context.Context, req *gnmipb.CapabilityRequest var supportedModels []gnmipb.ModelData dc, _ := sdc.NewTranslClient(nil, nil, ctx, extensions) supportedModels = append(supportedModels, dc.Capabilities()...) - dc, _ = sdc.NewMixedDbClient(nil, nil, "", gnmipb.Encoding_JSON_IETF, s.config.ZmqPort) + dc, _ = sdc.NewMixedDbClient(nil, nil, "", gnmipb.Encoding_JSON_IETF, s.config.ZmqPort, s.config.Vrf) supportedModels = append(supportedModels, dc.Capabilities()...) suppModels := make([]*gnmipb.ModelData, len(supportedModels)) diff --git a/sonic_data_client/client_test.go b/sonic_data_client/client_test.go index dcb4d74f..55cadaf4 100644 --- a/sonic_data_client/client_test.go +++ b/sonic_data_client/client_test.go @@ -793,17 +793,17 @@ func TestGetZmqClient(t *testing.T) { dpusTable.Hset("dpu0", "midplane_interface", "dpu0") dhcpPortTable.Hset("bridge-midplane|dpu0", "ips@", "127.0.0.2,127.0.0.1") - client, err := getZmqClient("dpu0", "") + client, err := getZmqClient("dpu0", "", "") if client != nil || err != nil { t.Errorf("empty ZMQ port should not get ZMQ client") } - client, err = getZmqClient("dpu0", "1234") + client, err = getZmqClient("dpu0", "1234", "") if client == nil { t.Errorf("get ZMQ client failed") } - client, err = getZmqClient("", "1234") + client, err = getZmqClient("", "1234", "") if client == nil { t.Errorf("get ZMQ client failed") } diff --git a/sonic_data_client/mixed_db_client.go b/sonic_data_client/mixed_db_client.go index ee9c8b07..4fe1bb22 100644 --- a/sonic_data_client/mixed_db_client.go +++ b/sonic_data_client/mixed_db_client.go @@ -159,10 +159,10 @@ func getZmqAddress(container string, zmqPort string) (string, error) { var zmqClientMap = map[string]swsscommon.ZmqClient{} -func getZmqClientByAddress(zmqAddress string) (swsscommon.ZmqClient, error) { +func getZmqClientByAddress(zmqAddress string, vrf string) (swsscommon.ZmqClient, error) { client, ok := zmqClientMap[zmqAddress] if !ok { - client = swsscommon.NewZmqClient(zmqAddress) + client = swsscommon.NewZmqClient(zmqAddress, vrf) zmqClientMap[zmqAddress] = client } @@ -181,7 +181,7 @@ func removeZmqClient(zmqClient swsscommon.ZmqClient) (error) { return fmt.Errorf("Can't find ZMQ client in zmqClientMap: %v", zmqClient) } -func getZmqClient(dpuId string, zmqPort string) (swsscommon.ZmqClient, error) { +func getZmqClient(dpuId string, zmqPort string, vrf string) (swsscommon.ZmqClient, error) { if zmqPort == "" { // ZMQ feature disabled when zmqPort flag not set return nil, nil @@ -189,7 +189,7 @@ func getZmqClient(dpuId string, zmqPort string) (swsscommon.ZmqClient, error) { if dpuId == sdcfg.SONIC_DEFAULT_CONTAINER { // When DPU ID is default, create ZMQ with local address - return getZmqClientByAddress("tcp://" + LOCAL_ADDRESS + ":" + zmqPort) + return getZmqClientByAddress("tcp://" + LOCAL_ADDRESS + ":" + zmqPort, vrf) } zmqAddress, err := getZmqAddress(dpuId, zmqPort) @@ -197,7 +197,7 @@ func getZmqClient(dpuId string, zmqPort string) (swsscommon.ZmqClient, error) { return nil, fmt.Errorf("Get ZMQ address failed: %v", err) } - return getZmqClientByAddress(zmqAddress) + return getZmqClientByAddress(zmqAddress, vrf) } // This function get target present in GNMI Request and @@ -493,7 +493,7 @@ func init() { initRedisDbMap() } -func NewMixedDbClient(paths []*gnmipb.Path, prefix *gnmipb.Path, origin string, encoding gnmipb.Encoding, zmqPort string) (Client, error) { +func NewMixedDbClient(paths []*gnmipb.Path, prefix *gnmipb.Path, origin string, encoding gnmipb.Encoding, zmqPort string, vrf string) (Client, error) { var err error // Initialize RedisDbMap for test @@ -556,7 +556,7 @@ func NewMixedDbClient(paths []*gnmipb.Path, prefix *gnmipb.Path, origin string, client.workPath = common_utils.GNMI_WORK_PATH // continer is DPU ID - client.zmqClient, err = getZmqClient(container, zmqPort) + client.zmqClient, err = getZmqClient(container, zmqPort, vrf) if err != nil { return nil, fmt.Errorf("Get ZMQ client failed: %v", err) } diff --git a/telemetry/telemetry.go b/telemetry/telemetry.go index 466cc33d..cb56e10c 100644 --- a/telemetry/telemetry.go +++ b/telemetry/telemetry.go @@ -57,6 +57,7 @@ type TelemetryConfig struct { WithMasterArbitration *bool WithSaveOnSet *bool IdleConnDuration *int + Vrf *string } func main() { @@ -165,6 +166,7 @@ func setupFlags(fs *flag.FlagSet) (*TelemetryConfig, *gnmi.Config, error) { WithMasterArbitration: fs.Bool("with-master-arbitration", false, "Enables master arbitration policy."), WithSaveOnSet: fs.Bool("with-save-on-set", false, "Enables save-on-set."), IdleConnDuration: fs.Int("idle_conn_duration", 5, "Seconds before server closes idle connections"), + Vrf: fs.String("vrf", "", "VRF name, when zmq_address belong on a VRF, need VRF name to bind ZMQ."), } fs.Var(&telemetryCfg.UserAuth, "client_auth", "Client auth mode(s) - none,cert,password") @@ -227,6 +229,7 @@ func setupFlags(fs *flag.FlagSet) (*TelemetryConfig, *gnmi.Config, error) { cfg.Threshold = int(*telemetryCfg.Threshold) cfg.IdleConnDuration = int(*telemetryCfg.IdleConnDuration) cfg.ConfigTableName = *telemetryCfg.ConfigTableName + cfg.Vrf = *telemetryCfg.Vrf // TODO: After other dependent projects are migrated to ZmqPort, remove ZmqAddress zmqAddress := *telemetryCfg.ZmqAddress From 470e2753db036ac8abcab975431c97f7810605f4 Mon Sep 17 00:00:00 2001 From: Zain Budhwani <99770260+zbud-msft@users.noreply.github.com> Date: Tue, 15 Oct 2024 00:43:25 -0700 Subject: [PATCH 3/4] Use version of gocov supported by sonic-gnmi (#300) Why I did it axw gocov was modified to use go 1.22 while currently sonic-gnmi still uses 1.13 a few days ago from this PR creation date. To ensure pipeline still works, we are specifying last stable version of axw gocov that still worked with sonic-gnmi pipeline which was 1.1.0 How I did it Specify gocov version How to verify it Pipeline --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8cc9c0ee..c711ee05 100644 --- a/Makefile +++ b/Makefile @@ -134,7 +134,7 @@ endif sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-data.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/sonic_data_client sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test -race -coverprofile=coverage-dbus.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/sonic_service_client sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(TESTENV) $(GO) test -race -coverprofile=coverage-translutils.txt -covermode=atomic -mod=vendor -v github.com/sonic-net/sonic-gnmi/transl_utils - $(GO) install github.com/axw/gocov/gocov@latest + $(GO) install github.com/axw/gocov/gocov@v1.1.0 $(GO) install github.com/AlekSi/gocov-xml@latest $(GO) mod vendor gocov convert coverage-*.txt | gocov-xml -source $(shell pwd) > coverage.xml From 743c7afac122d07701de9077150edd22b72a265c Mon Sep 17 00:00:00 2001 From: ganglv <88995770+ganglyu@users.noreply.github.com> Date: Wed, 16 Oct 2024 09:13:50 +0800 Subject: [PATCH 4/4] Update dbus timeout (#305) Why I did it dbus timeout is not enough for some hwsku. How I did it Increase dbus timeout to support all possible hwsku. How to verify it Run unit test and end2end test. --- sonic_service_client/dbus_client.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sonic_service_client/dbus_client.go b/sonic_service_client/dbus_client.go index 73af426c..48291570 100644 --- a/sonic_service_client/dbus_client.go +++ b/sonic_service_client/dbus_client.go @@ -104,7 +104,7 @@ func (c *DbusClient) ConfigReload(config string) error { busName := c.busNamePrefix + modName busPath := c.busPathPrefix + modName intName := c.intNamePrefix + modName + ".reload" - _, err := DbusApi(busName, busPath, intName, 10, config) + _, err := DbusApi(busName, busPath, intName, 60, config) return err } @@ -114,7 +114,7 @@ func (c *DbusClient) ConfigSave(fileName string) error { busName := c.busNamePrefix + modName busPath := c.busPathPrefix + modName intName := c.intNamePrefix + modName + ".save" - _, err := DbusApi(busName, busPath, intName, 10, fileName) + _, err := DbusApi(busName, busPath, intName, 60, fileName) return err } @@ -144,7 +144,7 @@ func (c *DbusClient) CreateCheckPoint(fileName string) error { busName := c.busNamePrefix + modName busPath := c.busPathPrefix + modName intName := c.intNamePrefix + modName + ".create_checkpoint" - _, err := DbusApi(busName, busPath, intName, 10, fileName) + _, err := DbusApi(busName, busPath, intName, 60, fileName) return err } @@ -154,7 +154,7 @@ func (c *DbusClient) DeleteCheckPoint(fileName string) error { busName := c.busNamePrefix + modName busPath := c.busPathPrefix + modName intName := c.intNamePrefix + modName + ".delete_checkpoint" - _, err := DbusApi(busName, busPath, intName, 10, fileName) + _, err := DbusApi(busName, busPath, intName, 60, fileName) return err } @@ -164,7 +164,7 @@ func (c *DbusClient) StopService(service string) error { busName := c.busNamePrefix + modName busPath := c.busPathPrefix + modName intName := c.intNamePrefix + modName + ".stop_service" - _, err := DbusApi(busName, busPath, intName, 90, service) + _, err := DbusApi(busName, busPath, intName, 240, service) return err } @@ -174,7 +174,7 @@ func (c *DbusClient) RestartService(service string) error { busName := c.busNamePrefix + modName busPath := c.busPathPrefix + modName intName := c.intNamePrefix + modName + ".restart_service" - _, err := DbusApi(busName, busPath, intName, 90, service) + _, err := DbusApi(busName, busPath, intName, 240, service) return err } @@ -184,7 +184,7 @@ func (c *DbusClient) GetFileStat(path string) (map[string]string, error) { busName := c.busNamePrefix + modName busPath := c.busPathPrefix + modName intName := c.intNamePrefix + modName + ".get_file_stat" - result, err := DbusApi(busName, busPath, intName, 10, path) + result, err := DbusApi(busName, busPath, intName, 60, path) if err != nil { return nil, err }