Hi, On 2/15/25 7:04 AM, Akihiko Odaki wrote: > tun simply advances iov_iter when it needs to pad virtio header, > which leaves the garbage in the buffer as is. This will become > especially problematic when tun starts to allow enabling the hash > reporting feature; even if the feature is enabled, the packet may lack a > hash value and may contain a hole in the virtio header because the > packet arrived before the feature gets enabled or does not contain the > header fields to be hashed. If the hole is not filled with zero, it is > impossible to tell if the packet lacks a hash value. Should virtio starting sending packets only after feature negotiation? In other words, can the above happen without another bug somewhere else? I guess the following question is mostly for Jason and Michael: could be possible (/would it make any sense) to use a virtio_net_hdr `flags` bit to explicitly signal the hash fields presence? i.e. making the actual virtio_net_hdr size 'dynamic'. > In theory, a user of tun can fill the buffer with zero before calling > read() to avoid such a problem, but leaving the garbage in the buffer is > awkward anyway so replace advancing the iterator with writing zeros. > > A user might have initialized the buffer to some non-zero value, > expecting tun to skip writing it. As this was never a documented > feature, this seems unlikely. > > The overhead of filling the hole in the header is negligible when the > header size is specified according to the specification as doing so will > not make another cache line dirty under a reasonable assumption. Below > is a proof of this statement: > > The first 10 bytes of the header is always written and tun also writes > the packet itself immediately after the > packet unless the packet is ^^^^^ this possibly should be 'virtio header'. Otherwise the sentence is hard to follow for me. > empty. This makes a hole between these writes whose size is: sz - 10 > where sz is the specified header size. > > Therefore, we will never make another cache line dirty when: > sz < L1_CACHE_BYTES + 10 > where L1_CACHE_BYTES is the cache line size. Assuming > L1_CACHE_BYTES >= 16, this inequation holds when: sz < 26. > > sz <= 20 according to the current specification so we even have a > margin of 5 bytes in case that the header size grows in a future version > of the specification. FTR, the upcoming GSO over UDP tunnel support will add other 4 bytes to the header. but that will still fit the given boundary. /P
