digitalocean/go-openvswitch

Fixed size of PortID in PortStat

hwchiu opened this issue · 4 comments

Hii.

The type of PortID in the PortStats is int and it's not a fixed length type.
We will meet the problems if we want to write that PortStats struct to buffer via the binary.Write.

I think we can fix that problem by changing the type from int to int32 to make the whole structure fixed length.

What do you think ?

Thanks.

type PortStats struct {
	// PortID specifies the OVS port ID which this PortStats refers to.
	PortID int

	// Received and Transmitted contain information regarding the number
	// of received and transmitted packets, bytes, etc.
	// OVS stores all of these counters as uint64 values.
	Received    PortStatsReceive
	Transmitted PortStatsTransmit
}

I am hesitant to make this change because we have used this field for years internally as an unsized integer like this. What is the benefit of being able to convert the structure using binary.Write?

Hi.
Thanks your reply.
I build a gRPC server/client in my environment and the server will reply the PortStats to the client.
Since I don't want to re-design a new structure like PortStats in my gRPC prototype, I use the type [][]byte to represent the data of PortStats and that's why I should use the binary.Write to convert the data from the well-known structure PortStats to a common type buffer.
In this case, sine the int isn't a fixed length filed and I can't use the binary.Write to convert the data.

Thanks.

I can't say I'd recommend doing that. Generally with gRPC, you want to create explicit message types for each type of data you're passing; an opaque bytes or repeated bytes type says absolutely nothing about the contents. On top of that, what's the endianness of those opaque bytes?

That said, if Open vSwitch uses some kind of fixed size integer internally to represent the types, it might be more reasonable to make the change. I'm guessing it does. This package doesn't intend to provide any kind of structure size or layout guarantees though; it's pretty high-level.

Got it, Thanks.