Feldman, Scott wrote:
Could the problem happen because I do the set-multi even if
the NIC is down? Is there enough locking in the
e1000_ethtool additions?
Change ETHTOOL_SETRXALL to work more like ETHTOOL_SRXCSUM, with the
check for netif_running and down/up or reset.
The set_multi is called from net/core/dev.c (or near-abouts) when
the promisc changes, and I'm quite sure that does not require the
NIC to be bounced.
So, if I really have to bounce the NIC to change the 'rx-all' flags,
then I'll have to move that code path out of the 'set-multi' method
and have it be a stand-alone method.
Or, is the problem just that I should NOT be twiddling any bits when
the NIC is down? Unless I'm confused, the only lock acquired when
changing PROMISC (which on e100 at least used to flush a bunch of the
rx-all flags, even though the receive logic threw them away later)
is the tx-lock.
For instance, would this perhaps be a better method:
static int e1000_ethtool_setrxall(struct net_device *netdev, uint32_t val) {
unsigned short old_flags = netdev->priv_flags;
if (val) {
netdev->priv_flags |= IFF_ACCEPT_ALL_FRAMES;
}
else {
netdev->priv_flags &= ~(IFF_ACCEPT_ALL_FRAMES);
}
/* printk("e1000_ethtool_setrxall (%s) val: %d\n",
netdev->name, val); */
if (old_flags != netdev->priv_flags) {
spin_lock_bh(&netdev->xmit_lock);
if (netif_running(netdev)) {
/*printk("Kicking e1000 for setrxall..\n");*/
e1000_set_multi(netdev);
} else {
/* Value will be flushed into the hardware when the
device is
* brought up.
*/
}
spin_unlock_bh(&netdev->xmit_lock);
}
return 0;
}
...
case ETHTOOL_SETRXALL: {
struct ethtool_value id;
if (copy_from_user(&id, addr, sizeof(id)))
return -EFAULT;
e1000_ethtool_setrxall(netdev, id.data);
return 0;
}
Thanks,
Ben
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
|