On Tue, Feb 21, 2023 at 09:36:06PM +0000, Parav Pandit wrote: > > So you are saying either live with the problem (this is best effort yes?) > Yes to best effort usage.
For sure something can be done to mitigate? How about randomizing the key for example? That's in just like 1 minute of thinking. I am guessing more can be done. > > > > > > > > > > +\item The user can observe the traffic information and enqueue > > > > > > +information > > > > > > of other normal > > > > > > + tunnels, and conduct targeted DoS attacks. > > > > > Once hash_report_tunnel_types is removed, this second attack is no > > > > > longer > > > > applicable. > > > > > Hence, please remove this too. > > > > > > > > > > > > ? > > > > I don't get how removing a field helps DoS. > > > > > > > I meant for the "observe and enqueue" part of the tunnel as not > > > applicable. > > > > Sorry still don't get it :( I don't know what is the "observe and enqueue" > > part of > > the tunnel and what is not applicable. But maybe Heng Qi does. > > > Tunnel type such as vxlan/gre etc is not placed in the virtio_net_hdr. > This way the net_hdr doesn't leak such information to upper layer drivers as > it cannot observe it. What is this information driver can't observe? It sees all the packets after all, we are not stripping tunneling headers. I also don't really know what are upper layer drivers - for sure layering of drivers is not covered in the spec for now so I am not sure what do you mean by that. The risk I mentioned is leaking the information *on the network*. > > > > \begin{lstlisting} struct virtio_net_rss_config { > > > > > > le32 hash_types; > > > > > > + le32 hash_tunnel_types; > > > > > This field is not needed as device config space advertisement for > > > > > the support > > > > is enough. > > > > > > > > > > If the intent is to enable hashing for the specific tunnel(s), an > > > > > individual > > > > command is better. > > > > > > > > new command? I am not sure why we want that. why not handle tunnels > > > > like we do other protocols? > > > > > > I didn't follow. > > > We probably discussed in another thread that to set M bits, it is wise to > > > avoid > > setting N other bits just to keep the command happy, where N >>> M and these > > N have a very strong relation in hw resource setup and packet steering. > > > Any examples of 'other protocols'? > > > > #define VIRTIO_NET_HASH_TYPE_IPv4 (1 << 0) > > #define VIRTIO_NET_HASH_TYPE_TCPv4 (1 << 1) > > #define VIRTIO_NET_HASH_TYPE_UDPv4 (1 << 2) > > > > this kind of thing. > > > > I don't see how a tunnel is different fundamentally. Why does it need its > > own > > field? > > Driver is in control to enable/disable tunnel based inner hash acceleration > only when its needed. > This way certain data path hw parsers can be enabled/disabled. > Without this it will be always enabled even if there may not be any user of > it. > Device has scope to optimize this flow. I feel you misunderstand the question. Or maybe I misunderstand what you are proposing. So tunnels need their own bits. But why a separate field and not just more bits along the existing ones? -- MST --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org