Re: [PATCH] hv: account for packet descriptor in maximum packet size

[Andrea Parri <parri.andrea@gmail.com>]

> > AFAICT, the above #define is fine, i.e., it represents an upper bound
> > on pkt_len as used in hv_pkt_iter_first() (this is all is required on
> > max_pkt_size, cf. the memcpy() in hv_pkt_iter_first()).
> >
> > The same consideration, AFAICT, holds for NETVSC_MAX_PKT_SIZE.
> >
> > The remarks about pkt_offset targetted the cases, such as hv_balloon,
> > where we can somehow upper bound
> >
> >         (pkt_len - pkt_offset)
> >
> > (the "packet payload"), since then an upper bound on pkt_offset would
> 
> I don't get it. Isn't it the same for storvsc? For storvsc we just
> have an upper bound of ("the packet payload") (pkt_len - pkt_offset)
> == sizeof(struct vstor_packet).
> 
> With more details:
> 
> drivers/scsi/storvsc_drv.c:storvsc_on_channel_callback:
> 
> foreach_vmbus_pkt(desc, channel) {
>         struct vstor_packet *packet = hv_pkt_data(desc);
> 
> where foreach_vmbus_pkt is a macro calling hv_pkt_iter_first, and
> hv_pkt_data is defined as:
> 
> /* Get data payload associated with descriptor */
> static inline void *hv_pkt_data(const struct vmpacket_descriptor *desc)
> {
>         return (void *)((unsigned long)desc + (desc->offset8 << 3));
> }
> 
> i.e. it expects that 'desc' points to a buffer at least
> '(desc->offset8 << 3) + sizeof(struct vstor_packet)' bytes long.
> 
> As Hyper-V is proprietary I can only guess what is the purpose of
> desc->offset8 (being forward compatible), so I agree with you that
> this is a real problem.
> Currently, Hyper-V only sends vmbus packets with offset8 == 2, so the
> expression above equals STORVSC_MAX_PKT_SIZE. If future Hyper-V
> somehow sends a packet with offset8 == 3, hv_storvsc certainly breaks.

It actually looks to me like we're on a same page.  ;) IOW, pkt_offset
is expected to be <= sizeof(struct vmpacket_descriptor) (=16 now) *for
storvsc.

With the risk of adding to the confusion, ;) pkt_offset is expected to
be > sizeof(struct vmpacket_descriptor) for netvsc (cf. the validation
of offset8 performed in netvsc_receive()).

  Andrea


> 
> Or, is it guaranteed that desc->offset8 will always be 2 and never
> change in future Hyper-V?
> 
> > give us an upper bound on pkt_len "for free" (associativity):
> >
> >         ptk_len = (pkt_len - pkt_offset) + pkt_offset
> >
> >   Andrea
> 
> Regards,
> Yanming