lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 8 May 2011 09:50:08 -0700 From: Mahesh Bandewar <maheshb@...gle.com> To: Michał Mirosław <mirq-linux@...e.qmqm.pl> Cc: Matt Carlson <mcarlson@...adcom.com>, David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>, Michael Chan <mchan@...adcom.com>, Tom Herbert <therbert@...gle.com> Subject: Re: [PATCHv5 2/2] tg3: Allow ethtool to enable/disable loopback. On Sat, May 7, 2011 at 12:43 AM, Michał Mirosław <mirq-linux@...e.qmqm.pl> wrote: > On Fri, May 06, 2011 at 11:18:37PM -0700, Mahesh Bandewar wrote: >> This patch adds tg3_set_features() to handle loopback mode. Currently the >> capability is added for the devices which support internal MAC loopback mode. >> So when enabled, it enables internal-MAC loopback. > [...] >> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c >> index 7c7c9a8..b7270c2 100644 >> --- a/drivers/net/tg3.c >> +++ b/drivers/net/tg3.c > [...] >> @@ -6319,6 +6356,24 @@ static u32 tg3_fix_features(struct net_device *dev, u32 features) >> return features; >> } >> >> +static int tg3_set_features(struct net_device *dev, u32 features) >> +{ >> + struct tg3 *tp = netdev_priv(dev); >> + u32 changed = dev->features ^ features; >> + >> + if (changed & NETIF_F_LOOPBACK) { >> + if (tg3_flag(tp, LOOPBACK_ENABLED)) >> + tg3_flag_clear(tp, LOOPBACK_ENABLED); >> + else >> + tg3_flag_set(tp, LOOPBACK_ENABLED); >> + >> + if (netif_running(dev)) >> + tg3_set_loopback(dev); >> + } >> + >> + return 0; >> +} >> + >> static inline void tg3_set_mtu(struct net_device *dev, struct tg3 *tp, >> int new_mtu) >> { >> @@ -9485,6 +9540,13 @@ static int tg3_open(struct net_device *dev) >> >> netif_tx_start_all_queues(dev); >> >> + /* >> + * Reset loopback feature if it was turned on while the device was down >> + * make sure that it's installed properly now. >> + */ >> + if (tg3_flag(tp, LOOPBACK_ENABLED)) >> + tg3_set_loopback(dev); >> + >> return 0; >> >> err_out3: > [...] > > So, you've just implemented what I said about enabling loopback at the end > of tg3_open(), but you also added (redundant) flag that mirrors > dev->features & NETIF_F_LOOPBACK. Why? > I had tried it before but it failed and I thought the device is not ready at the end of ndo_open call back, but in fact my change itself was causing the discrepancy in driver's private data structure. This I found while debugging the change and corrected by using tp->mac_mode for the current state of that reg instead of reading it from the reg into a local variable. I agree, as a side effect of this a redundant flag got created, which I'll remove. Thanks for pointing it out. Regards, --mahesh.. > Best Regards, > Michał Mirosław > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists