On Thu, Nov 07, 2013 at 08:59:22AM +0200, Michael S. Tsirkin wrote:
> On Tue, Nov 05, 2013 at 07:17:18PM +0800, Amos Kong wrote:
> > We currently just update the HMP NIC info when the last bit of macaddr
> > is written. This assumes that guest driver will write all the macaddr
> > from bit 0 to bit 5 when it changes the macaddr, this is the current
> > behavior of linux driver (e1000/rtl8139cp), but we can't do this
> > assumption.
> >
> > The macaddr that is used for rx-filter will be updated when every bit
> > is changed. This patch updates the e1000/rtl8139 nic to update HMP NIC
> > info when every bit is changed. It will be same as virtio-net.
> >
> > Signed-off-by: Amos Kong <ak...@redhat.com>
>
> I'm not sure I buy this.
>
> If we actually implement e.g. mac change notifications,
> sending them on writes of random bytes will confuse
> the host.
This patch just effects the monitor display of macaddr.
During each writing, the macaddr is used for rx-filter is really
changed.
In the real hardware, it supports to just write part of bits,
the rx-filtering is effected by every bit writing.
> I would say let's leave e1000/rtl8139 well alone unless
> we see guests that actually write mac without touching
> the last byte.
At least, linux rtl8139cp/e1000 writes macaddr from bit 0 to bit 5.
It works to just watch the last bit.
Thanks, Amos
> Then think of ways to detect when mac change is done
> for these.
>
> > ---
> > hw/net/e1000.c | 2 +-
> > hw/net/rtl8139.c | 5 +----
> > 2 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> > index ec8ecd7..2d60639 100644
> > --- a/hw/net/e1000.c
> > +++ b/hw/net/e1000.c
> > @@ -1110,7 +1110,7 @@ mac_writereg(E1000State *s, int index, uint32_t val)
> >
> > s->mac_reg[index] = val;
> >
> > - if (index == RA + 1) {
> > + if (index == RA || index == RA + 1) {
> > macaddr[0] = cpu_to_le32(s->mac_reg[RA]);
> > macaddr[1] = cpu_to_le32(s->mac_reg[RA + 1]);
> > qemu_format_nic_info_str(qemu_get_queue(s->nic), (uint8_t
> > *)macaddr);
> > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
> > index 5329f44..7f2b4db 100644
> > --- a/hw/net/rtl8139.c
> > +++ b/hw/net/rtl8139.c
> > @@ -2741,10 +2741,7 @@ static void rtl8139_io_writeb(void *opaque, uint8_t
> > addr, uint32_t val)
> >
> > switch (addr)
> > {
> > - case MAC0 ... MAC0+4:
> > - s->phys[addr - MAC0] = val;
> > - break;
> > - case MAC0+5:
> > + case MAC0 ... MAC0+5:
> > s->phys[addr - MAC0] = val;
> > qemu_format_nic_info_str(qemu_get_queue(s->nic), s->phys);
> > break;
> > --
> > 1.8.3.1
