netdev - Re: [PATCH] SMSC LAN9500 USB2.0 10/100 ethernet adapter driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200809081154.05512.david-b@pacbell.net>
Date:	Mon, 8 Sep 2008 11:54:05 -0700
From:	David Brownell <david-b@...bell.net>
To:	Steve Glendinning <steve.glendinning@...c.com>
Cc:	netdev@...r.kernel.org, Ian Saturley <ian.saturley@...c.com>,
	Catalin Marinas <catalin.marinas@....com>
Subject: Re: [PATCH] SMSC LAN9500 USB2.0 10/100 ethernet adapter driver

On Monday 08 September 2008, Steve Glendinning wrote:
> +
> +#define SMSC_WARNING(msg, args...) \
> +       printk(KERN_WARNING "%s: WARNING: " msg "\n", __func__, ## args)
> +
> +#ifdef USE_DEBUG
> +#define SMSC_TRACE(debug_bit, msg, args...) \
> +       if (debug_mode & debug_bit) { \
> +               printk(KERN_WARNING "%s: " msg "\n", __func__, ## args); \
> +       }
> +#else
> +#define SMSC_TRACE(dbgBit, msg, args...)   ({ do {} while (0); 0; })
> +#endif

Standard feedback for such stuff:

 - avoid printk() for diagnostics, use dev_*() driver model calls
 - ... or in this case, pr_warning() if you really must
 - provide better messages and avoid passing __func__ 
 - use inline functions instead of CPP macros
 - make dead code elimination remove debug code, not #ifdefs

And specific to network devices:

 - use the ethtool message level flags not private debug_mode

> +int turbo_mode = true;
> +module_param(turbo_mode, bool, 0);
> +MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");

I'm curious ... does this actually improve things on Linux?

My understanding is that on MS-Windows the costs of sending
a transaction through the USB stack are so high that their
APIs encourage such batching, and this has in some cases
extended down to wire protocols and (as seems true here)
even to hardware.

The approach Linux prefers is just to ensure that a queue
of transactions works efficiently ... so that for example
each Ethernet frame maps directly to one URB, and there's
no time wasted batching/unbatching.  (Neither in Linux,
nor in the peripheral.)

Last time I had any measurements on such issues, the
Linux stack outperformed MS-Windows in terms of per-packet
latency and also throughput, indicating that batching was
not necessary for performance on most network loads.  But
maybe that's not the case with your particular hardware,
or in certain significant/common use cases?

- Dave
--
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