[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 4/9] migration: Switch to using announce time
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v2 4/9] migration: Switch to using announce timer |
Date: |
Wed, 30 Jan 2019 11:41:24 -0500 |
On Wed, Jan 30, 2019 at 10:47:17AM +0000, Daniel P. Berrangé wrote:
> On Wed, Jan 30, 2019 at 10:32:31AM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > Switch the announcements to using the new announce timer.
> > Move the code that does it to announce.c rather than savevm
> > because it really has nothing to do with the actual migration.
> >
> > Migration starts the announce from bh's and so they're all
> > in the main thread/bql, and so there's never any racing with
> > the timers themselves.
> >
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> > include/migration/misc.h | 10 ------
> > include/net/announce.h | 2 ++
> > include/sysemu/sysemu.h | 2 --
> > migration/migration.c | 2 +-
> > migration/migration.h | 4 +++
> > migration/savevm.c | 72 ++--------------------------------------
> > migration/trace-events | 1 -
> > net/announce.c | 67 +++++++++++++++++++++++++++++++++++++
> > net/trace-events | 4 +++
> > 9 files changed, 81 insertions(+), 83 deletions(-)
>
>
> > +#ifndef ETH_P_RARP
> > +#define ETH_P_RARP 0x8035
> > +#endif
> > +#define ARP_HTYPE_ETH 0x0001
> > +#define ARP_PTYPE_IP 0x0800
> > +#define ARP_OP_REQUEST_REV 0x3
> > +
> > +static int announce_self_create(uint8_t *buf,
> > + uint8_t *mac_addr)
> > +{
> > + /* Ethernet header. */
> > + memset(buf, 0xff, 6); /* destination MAC addr */
> > + memcpy(buf + 6, mac_addr, 6); /* source MAC addr */
> > + *(uint16_t *)(buf + 12) = htons(ETH_P_RARP); /* ethertype */
> > +
> > + /* RARP header. */
> > + *(uint16_t *)(buf + 14) = htons(ARP_HTYPE_ETH); /* hardware addr space
> > */
> > + *(uint16_t *)(buf + 16) = htons(ARP_PTYPE_IP); /* protocol addr space
> > */
> > + *(buf + 18) = 6; /* hardware addr length (ethernet) */
> > + *(buf + 19) = 4; /* protocol addr length (IPv4) */
> > + *(uint16_t *)(buf + 20) = htons(ARP_OP_REQUEST_REV); /* opcode */
> > + memcpy(buf + 22, mac_addr, 6); /* source hw addr */
> > + memset(buf + 28, 0x00, 4); /* source protocol addr */
> > + memcpy(buf + 32, mac_addr, 6); /* target hw addr */
> > + memset(buf + 38, 0x00, 4); /* target protocol addr */
> > +
> > + /* Padding to get up to 60 bytes (ethernet min packet size, minus
> > FCS). */
> > + memset(buf + 42, 0x00, 18);
> > +
> > + return 60; /* len (FCS will be added by hardware) */
> > +}
>
> I think I would suggest defining a packed struct for the ethernet
> packet we're sending, so we can just reference named fields instead
> of magic offsets. Then I realized this patch is just moving some
> pre-existing code, so it is probably better to leave it as it is
> in this patch, as mixing code movement with refactoring is bad
> practice. If anyone fancies doing a latter patch to use a struct
> though, it might be nice.
Won't work - this need to be network endian and endian-ness is all
messed up with packed structs.
The place to fix this would be in the C standard :)
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- [Qemu-devel] [PATCH v2 1/9] net: Introduce announce timer, (continued)
[Qemu-devel] [PATCH v2 3/9] virtio-net: Switch to using announce timer, Dr. David Alan Gilbert (git), 2019/01/30
[Qemu-devel] [PATCH v2 4/9] migration: Switch to using announce timer, Dr. David Alan Gilbert (git), 2019/01/30
[Qemu-devel] [PATCH v2 5/9] net: Add a network device specific self-announcement ability, Dr. David Alan Gilbert (git), 2019/01/30
[Qemu-devel] [PATCH v2 6/9] virtio-net: Allow qemu_announce_self to trigger virtio announcements, Dr. David Alan Gilbert (git), 2019/01/30
[Qemu-devel] [PATCH v2 7/9] qmp: Add announce-self command, Dr. David Alan Gilbert (git), 2019/01/30
[Qemu-devel] [PATCH v2 8/9] hmp: Add hmp_announce_self, Dr. David Alan Gilbert (git), 2019/01/30
[Qemu-devel] [PATCH v2 9/9] tests: Add a test for qemu self announcements, Dr. David Alan Gilbert (git), 2019/01/30