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: Sat, 3 May 2014 13:40:29 +0200 From: Francois Romieu <romieu@...zoreil.com> To: Darek Marcinkiewicz <reksio@...term.pl> Cc: davem@...emloft.net, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v5 1/1] Driver for Beckhoff CX5020 EtherCAT master module. Darek Marcinkiewicz <reksio@...term.pl> : [...] > This driver adds support for EtherCAT master module located on CCAT > FPGA found on Beckhoff CX series industrial PCs. The driver exposes > EtherCAT master as an ethernet interface. > > EtherCAT is a filedbus protocol defined on top of ethernet and Beckhoff s/filedbus/fieldbus/ :o) > CX5020 PCs come with built-in EtherCAT master module located on a FPGA, > which in turn is connected to a PCI bus. > > Signed-off-by: Dariusz Marcinkiewicz <reksio@...term.pl> I have attached four patches. Patch #3 / 4 contains a bugfix. Remaining patches are small nits. The points below are not showstoppers. Your answer will be welcome though. If the tx_desc_free list can turn empty, I don't see much point for not using an array as descriptor ring. It's quite common for linux ethernet drivers. Be it a list or an array, sizing it as the max count of descriptors that could fit in the memory window through which packet descriptors _and_ ethernet data are copied seems overkill. It may be an upper bound though. If you are not sure about a sane default value, ethtool API allows to change the descriptor ring size. Given the amount of debug messages, ethtool registers dump support could be of some value. I guess that the ASIC and the host CPU need some external polling to communicate (no IRQ, really ?), whence the hrtimer. It should naturally fit with NAPI. hrtimer could thus be disabled as soon as some ASIC event is detected. It should save some hrtimer work when load increases and the usual Tx lock avoidance patterns would kick in (current locking is gross). Did Beckhoff document their FPGA PCI interface ? -- Ueimor View attachment "0001-ec_bhf-inverted-xmas-tree.patch" of type "text/plain" (1446 bytes) View attachment "0002-ec_bhf-line-breaks-and-indent.patch" of type "text/plain" (4203 bytes) View attachment "0003-ec_bhf-rx_desc-vs-tx_desc-confusion.patch" of type "text/plain" (1249 bytes) View attachment "0004-ec_bhf-use-once-variable-removal.patch" of type "text/plain" (1548 bytes)
Powered by blists - more mailing lists