Patrick McHardy said the following on 2008-6-23 21:47:

quoted

quoted

quoted

@@ -5528,6 +5529,7 @@ de4x5_ioctl(struct net_device *dev, struct ifreq

*rq, int cmd)
     omr &= ~OMR_PR;
     outl(omr, DE4X5_OMR);
     dev->flags &= ~IFF_PROMISC;
+    dev->promiscuity = 0;
     break;

Shouldn't this be using dev_set_promiscuity().

I actually meant dev_change_flags(), sorry.

dev_change_flags() can not completely change flag IFF_PROMISC like
IFF_UP, etc.
So dev_change_flags() has no big difference to dev_set_promiscuity().
I think dev_set_promiscuity() is suitable here.

quoted

No.
1. dev_set_promiscuity do
  a. set/unset IFF_PROMISC
  b. promiscuity++/--
  c. audit
  d. dev_set_rx_mode (upload unicast and multicast list to device)
  Here, in ioctl, a & b is enough.

Auditing should certainly be done if promiscous mode is set.
Calling dev_set_rx_mode doesn't hurt, even if it does the ioctl
handler could be changed not to care. Besides this is neither
taking the rtnl_mutex as required nor sending notifcations
to userspace.

Agree.

quoted

2. dev->flags unset IFF_PROMISC and dev->promiscuity = 0 can not be
   replaced by dev_set_promiscuity(). Because, we don't decrease
   promiscuity here, but set promiscuity zero for unset IFF_PROMISC.

And that looks like a bug, the driver shouldn't disable
promiscuity if something still requires it.

It's hard to say that.
In theory, user-space can require device to disable promisc by driver's
ioctl. 
But OTOH if something else still want device to be promisc, user and
driver have no method to let them decrease the refcnt promiscuity. Because
promiscuity decrement is initiative action from upper layer, drivers don't
know who need promiscuity.

Humm, tired, go to sleep and figure out how to do after refreshing. :)