The number tag of config_type fields in Filter proto can not exceeds 3.

[yuanqijing]

Please provide an in-depth description of the question you have: Hi, I was trying to add a local_rate_limit field inside the Filter message.

message Filter {
  string name = 1;
  oneof config_type {
    filter.TcpProxy tcp_proxy = 2;
    filter.HttpConnectionManager http_connection_manager = 3;
    filter.LocalRateLimit local_rate_limit = 4;
  }
}

After running make gen, the config_type_case field obtained in eBPF is 0, but I expect it to be 4. Additionally, I noticed that if the number tag of any fields is set to >= 4, config_type_case always shows 0 in C side. I would like to know how to solve this or this is a bug. Below is a test code to reproduce it.

apiFilter := &listener_v2.Filter{}
fn := func() {
  buf, err := proto.Marshal(apiFilter)
  if err != nil {
    log.Errorf("proto.Marshal failed: %v", err)
    return
  }
  convertToPack := func(buf []byte) *C.uint8_t {
    return (*C.uint8_t)(unsafe.Pointer(&buf[0]))
  }
  cmsg := C.listener__filter__unpack(nil, C.size_t(len(buf)), convertToPack(buf))
  klog.Infof("listener__filter__unpack: %v", spew.Sdump(cmsg))
}
klog.Infof("test local rate limit")
apiFilter.ConfigType = &listener_v2.Filter_LocalRateLimit{}
fn()
klog.Infof("test tcp proxy")
apiFilter.ConfigType = &listener_v2.Filter_TcpProxy{}
fn()
klog.Infof("test http connection manager")
apiFilter.ConfigType = &listener_v2.Filter_HttpConnectionManager{}
fn()

What do you think about this question?:

Environment:

• Kmesh version:
• Others:

[nlgwcy]

You can view the xDS models supported by kmesh in the code repository. Currently, kmesh supports filter.TcpProxy and filter.HttpConnectionManager.

[yuanqijing]

You can view the xDS models supported by kmesh in the code repository. Currently, kmesh supports filter.TcpProxy and filter.HttpConnectionManager.

yes, i was trying to enhance kmesh with ratelimit feature, describe in https://www.envoyproxy.io/docs/envoy/latest/configuration/listeners/network_filters/local_rate_limit_filter the config should place in Filter proto. But i found some problems(may be a bug) here.

[yuanqijing]

The problem i want to know here is "the number tag of any fields is set to >= 4, it always shows 0 on ebpf side".

[yuanqijing]

@nlgwcy is there any documentation / instruction on how to add a new field ? i just define a new field, make gen and add struct in lds cache, what else should i do?

[tacslon]

@yuanqijing Have you tried to add some logs in newApiFilterAndRouteName() to see if it works properly?

[nlgwcy]

Proto has a grammar error? I can add LocalRateLimit to tcp_proxy.proto.

// tcp_proxy.proto
package filter;
option go_package = "kmesh.net/kmesh/api/filter;filter";
message LocalRateLimit {
  string share_key = 4;
}

message TcpProxy {
  // cluster based on weights.
  message WeightedCluster {

// listener_components.proto
message Filter {
  string name = 1;
  oneof config_type {
    filter.TcpProxy tcp_proxy = 2;
    filter.HttpConnectionManager http_connection_manager = 3;
    filter.LocalRateLimit local_rate_limit = 4;
  }
}

make gen: api/v2-c/listener/listener_components.pb-c.h

typedef enum {
  LISTENER__FILTER__CONFIG_TYPE__NOT_SET = 0,
  LISTENER__FILTER__CONFIG_TYPE_TCP_PROXY = 2,
  LISTENER__FILTER__CONFIG_TYPE_HTTP_CONNECTION_MANAGER = 3,
  LISTENER__FILTER__CONFIG_TYPE_LOCAL_RATE_LIMIT = 4
    PROTOBUF_C__FORCE_ENUM_TO_BE_INT_SIZE(LISTENER__FILTER__CONFIG_TYPE__CASE)
} Listener__Filter__ConfigTypeCase;

[yuanqijing]

@nlgwcy @tacslon Thanks, still not work, i will dig more into this, and provide more information here.

[tacslon]

I have reproduced in oneof config_type of message Filter, also in oneof cluster_specifier in message RouteAction by adding a new field:

  oneof cluster_specifier {
    // Indicates the upstream cluster to which the request should be routed to.
    string cluster = 1;
    // Multiple upstream clusters can be specified for a given route. The
    // request is routed to one of the upstream clusters based on weights
    // assigned to each cluster.
    WeightedCluster weighted_clusters = 3;
+   string test = 4;
  }

Then print the case is 0, which should be 4:

cluster_specifier_case: (main._Ctype_Route__RouteAction__ClusterSpecifierCase) 0,

[tacslon]

Try to update protoc-c verison to 1.5.0. If still not works, see what envoy does https://github.com/envoyproxy/envoy/blob/main/api/contrib/envoy/extensions/filters/network/sip_proxy/tra/v3alpha/tra.proto

[yuanqijing]

Try to update protoc-c verison to 1.5.0. If still not works, see what envoy does https://github.com/envoyproxy/envoy/blob/main/api/contrib/envoy/extensions/filters/network/sip_proxy/tra/v3alpha/tra.proto

upgrade protoc-c verison to 1.5.0 still not works.

[hzxuzhonghu]

message Filter {
  string name = 1;
  oneof config_type {
    filter.TcpProxy tcp_proxy = 2;
    filter.HttpConnectionManager http_connection_manager = 3;
    filter.LocalRateLimit local_rate_limit = 4;
  }
}

Where do you define LocalRateLimit , i have changed the filter.LocalRateLimit to filter.HttpConnectionManager or simply string, make gen succeeded

[yuanqijing]

message Filter {
  string name = 1;
  oneof config_type {
    filter.TcpProxy tcp_proxy = 2;
    filter.HttpConnectionManager http_connection_manager = 3;
    filter.LocalRateLimit local_rate_limit = 4;
  }
}

Where do you define LocalRateLimit , i have changed the filter.LocalRateLimit to filter.HttpConnectionManager or simply string, make gen succeeded

Where do you define LocalRateLimit \=\=\= I defined in api/filter/local_ratelimit.proto. Here is my definition:

syntax = "proto3";

package filter;
option go_package = "kmesh.net/kmesh/api/filter;filter";

message LocalRateLimit {
  int64 bucket = 1;
}

make gen succeeded \=\=\= In my case make gen also succeeded but the data from GO to C is not correct, you can verify it through replace the code below in daemon/main.go:

package main

// #cgo pkg-config: api-v2-c
// #include "deserialization_to_bpf_map.h"
// #include "listener/listener.pb-c.h"
import "C"
import (
    "github.com/davecgh/go-spew/spew"
    "google.golang.org/protobuf/proto"
    "k8s.io/klog"
    "time"
    "unsafe"

    listener_v2 "kmesh.net/kmesh/api/v2/listener"
)

func main() {
    // debug
    dig := func(msg proto.Message) {
        buf, err := proto.Marshal(msg)
        if err != nil {
            klog.Errorf("proto.Marshal failed")
            panic(err)
        }
        cMsg := C.listener__filter__unpack(nil, C.size_t(len(buf)), (*C.uint8_t)(unsafe.Pointer(&buf[0])))
        klog.Infof("cMsg: %v", spew.Sdump(cMsg))
    }
    test := &listener_v2.Filter{}
    test.ConfigType = &listener_v2.Filter_LocalRateLimit{}
    dig(test)

    test.ConfigType = &listener_v2.Filter_TcpProxy{}
    dig(test)

    test.ConfigType = &listener_v2.Filter_HttpConnectionManager{}
    dig(test)

    time.Sleep(60 * time.Second)
}

it outputs cluster_specifier_case: (main._Ctype_Route__RouteAction__ClusterSpecifierCase) 0 for case LocalRateLImit.

[yuanqijing]

@tacslon I think i found the reason, the cgo using gcc to compile *.h files, where the libkmesh_api_v2_c.so using clang which may have conflicts. I compile daemon/go using CC=clang CXX=clang++ the problem solved.

root@vagrant:/app# docker run -it ghcr.io/kmesh-net/kmesh-build:latest sh
sh-5.2# go env
...
CC='gcc'
CXX='g++'

[Okabe-Rintarou-0]

would you write a fix pr? I've also met this problem.

[yuanqijing]

would you write a fix pr? I've also met this problem.

sure, i'll create a pr after testing on my machine.

[yuanqijing]

@tacslon @hzxuzhonghu hi, i have create a pr #836, can anyone review on this ?