diff mbox

[v2,1/5] net: Support ethtool ops for rx of errored frames.

Message ID 1308430045-24816-2-git-send-email-greearb@candelatech.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Ben Greear June 18, 2011, 8:47 p.m. UTC 
From: Ben Greear <greearb@candelatech.com>

This can be useful when sniffing dodgy networks.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 675ddc0... fb0fac9... M	include/linux/ethtool.h
:100644 100644 0e01860... 443a63c... M	net/core/ethtool.c
 include/linux/ethtool.h |    7 +++++++
 net/core/ethtool.c      |    8 ++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

Comments

Francois Romieu June 18, 2011, 9:34 p.m. UTC | #1 
greearb@candelatech.com <greearb@candelatech.com> :
[...]
> This can be useful when sniffing dodgy networks.

Do you plan to add something similar - i.e. not per packet - for the Tx path ?
Ben Greear June 19, 2011, 8:11 p.m. UTC | #2 
On 06/18/2011 02:34 PM, Francois Romieu wrote:
> greearb@candelatech.com<greearb@candelatech.com>  :
> [...]
>> This can be useful when sniffing dodgy networks.
>
> Do you plan to add something similar - i.e. not per packet - for the Tx path ?

Ability to tx errored frames?  I posted a patch to enable sending
frames with custom (ie, invalid) Ethernet FCS, but you have to
enable it per-socket, and it will only work with AF_PACKET sockets.

Can you offer more details on what you are asking for?

Thanks,
Ben
Francois Romieu June 19, 2011, 10:48 p.m. UTC | #3 
Ben Greear <greearb@candelatech.com> :
> On 06/18/2011 02:34 PM, Francois Romieu wrote:
> >greearb@candelatech.com<greearb@candelatech.com>  :
> >[...]
> >>This can be useful when sniffing dodgy networks.
> >
> >Do you plan to add something similar - i.e. not per packet - for the Tx path ?
> 
> Ability to tx errored frames?  I posted a patch to enable sending
> frames with custom (ie, invalid) Ethernet FCS, but you have to
> enable it per-socket, and it will only work with AF_PACKET sockets.
> 
> Can you offer more details on what you are asking for?

Realtek's 816[89] and 810[23] have no room in their Tx descriptors to
control ethernet checksum generation. There is a global register (TxConfig)
though. Before looking at the per-socket information, the driver would need
to be instructed to disable Tx hardware ethernet checksumming globally.
Ben Greear June 19, 2011, 11:15 p.m. UTC | #4 
On 06/19/2011 03:48 PM, Francois Romieu wrote:
> Ben Greear<greearb@candelatech.com>  :
>> On 06/18/2011 02:34 PM, Francois Romieu wrote:
>>> greearb@candelatech.com<greearb@candelatech.com>   :
>>> [...]
>>>> This can be useful when sniffing dodgy networks.
>>>
>>> Do you plan to add something similar - i.e. not per packet - for the Tx path ?
>>
>> Ability to tx errored frames?  I posted a patch to enable sending
>> frames with custom (ie, invalid) Ethernet FCS, but you have to
>> enable it per-socket, and it will only work with AF_PACKET sockets.
>>
>> Can you offer more details on what you are asking for?
>
> Realtek's 816[89] and 810[23] have no room in their Tx descriptors to
> control ethernet checksum generation. There is a global register (TxConfig)
> though. Before looking at the per-socket information, the driver would need
> to be instructed to disable Tx hardware ethernet checksumming globally.

Ok.  I think I'll take a stab at adding a more general low-level-flags
API to ethtool, with a bitfield that can be used to twiddle these sorts
of things.  That way, we won't have to add new methods every time we
poke a new feature in.

This would be separate from the 'features' patches that are floating around.

Thanks,
Ben
David Miller June 19, 2011, 11:24 p.m. UTC | #5 
From: Ben Greear <greearb@candelatech.com>
Date: Sun, 19 Jun 2011 16:15:56 -0700

> This would be separate from the 'features' patches that are floating
> around.

netdev->features is how you should implement your patch, please don't
create new facilities.

With netdev->features they can be validated against netdev->hw_features
which, in this case, r8169 would have this feature bit clear.

That's how all of this stuff is designed to work, the driver says what
the hardware can do and thus ethtool generically can validate attempts
to turn on features.

No new facilities are necessary at all.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 675ddc0..fb0fac9 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -880,6 +880,9 @@  bool ethtool_invalid_flags(struct net_device *dev, u32 data, u32 supported);
  * @set_save_rxfcs:  Set flag to save (1), or discard (0), the Ethernet
  *	Frame Checksum for received packets.
  * @get_save_rxfcs:  Get current value for Save RX-FCS flag.
+ * @set_save_rxerr:  Set flag to save (1), or discard (0), received Ethernet
+ *	frames with errors (bad checksum, etc)
+ * @get_save_rxerr:  Get current value for Save RX-ERR flag.
  *
  * All operations are optional (i.e. the function pointer may be set
  * to %NULL) and callers must take this into account.  Callers must
@@ -960,6 +963,8 @@  struct ethtool_ops {
 	int	(*set_dump)(struct net_device *, struct ethtool_dump *);
 	int	(*set_save_rxfcs)(struct net_device *, u32);
 	u32	(*get_save_rxfcs)(struct net_device *);
+	int	(*set_save_rxerr)(struct net_device *, u32);
+	u32	(*get_save_rxerr)(struct net_device *);
 
 };
 #endif /* __KERNEL__ */
@@ -1036,6 +1041,8 @@  struct ethtool_ops {
 #define ETHTOOL_GET_DUMP_DATA	0x00000040 /* Get dump data */
 #define ETHTOOL_GET_SAVE_RXFCS	0x00000041 /* Get RX Save Frame Checksum */
 #define ETHTOOL_SET_SAVE_RXFCS	0x00000042 /* Set RX Save Frame Checksum */
+#define ETHTOOL_GET_SAVE_RXERR	0x00000043 /* Get RX Save Errored Frames */
+#define ETHTOOL_SET_SAVE_RXERR	0x00000044 /* Set RX Save Errored Frames */
 
 
 /* compatibility with older code */
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 0e01860..443a63c 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -2160,6 +2160,14 @@  int dev_ethtool(struct net *net, struct ifreq *ifr)
 		rc =  ethtool_get_value(dev, useraddr, ethcmd,
 					dev->ethtool_ops->get_save_rxfcs);
 		break;
+	case ETHTOOL_SET_SAVE_RXERR:
+		rc = ethtool_set_value(dev, useraddr,
+				       dev->ethtool_ops->set_save_rxerr);
+		break;
+	case ETHTOOL_GET_SAVE_RXERR:
+		rc =  ethtool_get_value(dev, useraddr, ethcmd,
+					dev->ethtool_ops->get_save_rxerr);
+		break;
 	default:
 		rc = -EOPNOTSUPP;
 	}