Re: [PATCH net-next 4/9] ethtool: add tunnel info interface

[Michal Kubecek <mkubecek@suse.cz>]

On Wed, Jul 08, 2020 at 04:30:49PM -0700, Jakub Kicinski wrote:
> > > +	ETHTOOL_A_TUNNEL_UDP_TABLE_ENTRY,		/* nest - _UDP_ENTRY_* */
> > > +
> > > +	/* add new constants above here */
> > > +	__ETHTOOL_A_TUNNEL_UDP_TABLE_CNT,
> > > +	ETHTOOL_A_TUNNEL_UDP_TABLE_MAX = (__ETHTOOL_A_TUNNEL_UDP_TABLE_CNT - 1)
> > > +};
> > > +
> > > +enum {
> > > +	ETHTOOL_A_TUNNEL_UDP_ENTRY_UNSPEC,
> > > +
> > > +	ETHTOOL_A_TUNNEL_UDP_ENTRY_PORT,		/* be16 */  
> > 
> > Do we get some benefit from passing the port in network byte order? It
> > would be helpful if we expected userspace to copy it e.g. into struct
> > sockaddr_in but that doesn't seem to be the case.

Let's leave it as be16 for consistency then.

> > How big can the message get? Can we be sure the information for one
> > device will always fit into a reasonably sized message? Attribute
> > ETHTOOL_A_TUNNEL_INFO_UDP_PORTS is limited by 65535 bytes (attribute
> > size is u16), can we always fit into this size?
> 
> I don't think I've seen any driver with more than 2 tables 
> or 16 entries total, and they don't seem to be growing in newer
> HW (people tend to use standard ports).
> 
> 188B + 16 * 20B = 508B - so we should be pretty safe with 64k.

So we are safe even if things grow by a factor of 100. Sounds good
enough, thanks.

Michal