search

openconfig / grpctunnel

A TCP-over-gRPC Tunnel
Apache License 2.0
81 stars 24 forks source link

TargetType is not used in message target #16

Closed dliu2223 closed 3 years ago

dliu2223 commented 3 years ago

Please explain why target_type is a string instead of the enum TargetType

Thanks

David

enum TargetType { UNKNOWN = 0; // https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?search=22 SSH = 22; // https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?search=9339 GNMI_GNOI = 9339; } message Target { enum TargetOp { UNKNOWN = 0; ADD = 1; REMOVE = 2; } TargetOp op = 1; // Used to ack the registration of target_id and target_type. bool accept = 2; // Target id identifies which handler to use for a tunnel stream. string target_id = 3; // String value of the corresponding TargetType for a standard protocol. // A non-enumerated protocol is supported so long as both tunnel client and // server are in agreement on a particular value. string target_type = 4; string error = 5; }

gcsl commented 3 years ago

The enum allows for retrieving a string constant for a set of defined types. e.g. https://github.com/openconfig/gnmi/blob/master/testing/fake/gnmi/agent.go#L89

However, by allowing the target type to be a string, an operator could also tunnel other protocols not in the enum so long as the client and server agreed on common type strings.

On Mon, Feb 15, 2021 at 3:15 PM dliu2223 notifications@github.com wrote:

Please explain why target_type is a string instead of the enum TargetType

Thanks

David

enum TargetType { UNKNOWN = 0; // https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?search=22 SSH = 22; // https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?search=9339 GNMI_GNOI = 9339; } message Target { enum TargetOp { UNKNOWN = 0; ADD = 1; REMOVE = 2; } TargetOp op = 1; // Used to ack the registration of target_id and target_type. bool accept = 2; // Target id identifies which handler to use for a tunnel stream. string target_id = 3; // String value of the corresponding TargetType for a standard protocol. // A non-enumerated protocol is supported so long as both tunnel client and // server are in agreement on a particular value. string target_type = 4; string error = 5; }

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openconfig/grpctunnel/issues/16, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEL4QL7I2L5ICJMDPLHDA6LS7F6GRANCNFSM4XVIDCIQ .

dliu2223 commented 3 years ago

Thank you for the explanation. I was planning to extend the enum to support OpenFlow and P4 for data plane programming through gRPC tunnel, will switch to string for now, maybe submit changes in the future when implementation is done.

David

gcsl commented 3 years ago

The intention is to allow for expansion of the enum and both P4 and Openflow would be potential candidates.

On Tue, Feb 16, 2021 at 2:44 PM dliu2223 notifications@github.com wrote:

Thank you for the explanation. I was planning to extend the enum to support OpenFlow and P4 for data plane programming through gRPC tunnel, will switch to string for now, maybe submit changes in the future when implementation is done.

David

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openconfig/grpctunnel/issues/16#issuecomment-780074861, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEL4QL54EYCKWES52CES63DS7LDLFANCNFSM4XVIDCIQ .