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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> > Provided that there are indeed drivers (hv_storvsc and hv_netvsc)
> > which explicitly account for vmpacket_descriptor header, changing
> > max_pkt_size for individual drivers makes more sense.
> > However in this case I'm not sure about our reasoning of 'pkt_offset'
> > above. In drivers/scsi/storvsc_drv.c:
> >
> > #define STORVSC_MAX_PKT_SIZE (sizeof(struct vmpacket_descriptor) +\
> >                               sizeof(struct vstor_packet))
> >
> > Should I also change this 'sizeof(struct vmpacket_descriptor)' to
> > VMBUS_MAX_PKT_DESCR_SIZE? Otherwise this would not match the check in
> > hv_pkt_iter_first.
>
> 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.

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



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux