* Re: [E1000-devel] [PATCH] e100: Fix inconsistency in bad frames handling
[not found] <BANLkTimpYniHhN6ccMXN7Lx3xDdK6sC+FQ@mail.gmail.com>
@ 2011-06-06 17:49 ` Brandeburg, Jesse
2011-06-06 17:56 ` Ben Greear
0 siblings, 1 reply; 9+ messages in thread
From: Brandeburg, Jesse @ 2011-06-06 17:49 UTC (permalink / raw)
To: Andrea Merello; +Cc: e1000-devel@lists.sourceforge.net, netdev
<added netdev>, removed other useless lists.
On Sat, 4 Jun 2011, Andrea Merello wrote:
> In e100 driver it seems that the intention was to accept bad frames in
> promiscuous mode and loopback mode.
> I think this is evident because of the following code in the driver:
>
> if (nic->flags & promiscuous || nic->loopback) {
> config->rx_save_bad_frames = 0x1; /* 1=save, 0=discard */
> config->rx_discard_short_frames = 0x0; /* 1=discard, 0=save */
> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
> }
>
Hi, thanks for your work on e100.
> However this intention is not really realized because bad frames are
> discarded later by SW check.
> This patch finally honors the above intention, making the RX code to
> let bad frames to pass when the NIC is in promiscuous or loopback
> mode.
I think this may be a mistake by the authors of the software developers
manual. The manual suggests that save bad frames and save short frames
should be enabled in promisc mode, but all of our other drivers *do not*
save bad frames when in promiscuous mode (by design). This is intentional
because a bad frame is just that, bad, and with no hope of knowing if the
data in it is okay/malicious/other. I understand your reasoning above,
but realistically the rx_save_bad_frames should NOT be set. I'd ack a
patch to comment that line out.
> This helped me a lot to debug an FPGA ethernet core.
> Maybe it can be also useful to someone else..
I think this patch is just that, debug only. As a developer I understand
why this is useful, but there is no reason any normal user would be able
to benefit from this, so for now, sorry:
NACK.
>
> Thanks
> Andrea
>
> --- drivers/net/e100_orig.c 2011-06-14 23:29:38.322267075 +0200
> +++ drivers/net/e100.c 2011-06-14 23:34:10.700791472 +0200
> @@ -1975,7 +1975,8 @@ static int e100_rx_indicate(struct nic *
> skb_put(skb, actual_size);
> skb->protocol = eth_type_trans(skb, nic->netdev);
>
> - if (unlikely(!(rfd_status & cb_ok))) {
> + if (unlikely(!(nic->flags & promiscuous || nic->loopback) &&
> + !(rfd_status & cb_ok))) {
> /* Don't indicate if hardware indicates errors */
> dev_kfree_skb_any(skb);
> } else if (actual_size > ETH_DATA_LEN + VLAN_ETH_HLEN) {
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] e100: Fix inconsistency in bad frames handling
2011-06-06 17:49 ` [E1000-devel] [PATCH] e100: Fix inconsistency in bad frames handling Brandeburg, Jesse
@ 2011-06-06 17:56 ` Ben Greear
2011-06-06 20:15 ` [E1000-devel] " Ben Hutchings
0 siblings, 1 reply; 9+ messages in thread
From: Ben Greear @ 2011-06-06 17:56 UTC (permalink / raw)
To: Brandeburg, Jesse
Cc: Andrea Merello, e1000-devel@lists.sourceforge.net, netdev
On 06/06/2011 10:49 AM, Brandeburg, Jesse wrote:
>
> <added netdev>, removed other useless lists.
>
> On Sat, 4 Jun 2011, Andrea Merello wrote:
>> In e100 driver it seems that the intention was to accept bad frames in
>> promiscuous mode and loopback mode.
>> I think this is evident because of the following code in the driver:
>>
>> if (nic->flags& promiscuous || nic->loopback) {
>> config->rx_save_bad_frames = 0x1; /* 1=save, 0=discard */
>> config->rx_discard_short_frames = 0x0; /* 1=discard, 0=save */
>> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
>> }
>>
>
> Hi, thanks for your work on e100.
>
>> However this intention is not really realized because bad frames are
>> discarded later by SW check.
>> This patch finally honors the above intention, making the RX code to
>> let bad frames to pass when the NIC is in promiscuous or loopback
>> mode.
>
> I think this may be a mistake by the authors of the software developers
> manual. The manual suggests that save bad frames and save short frames
> should be enabled in promisc mode, but all of our other drivers *do not*
> save bad frames when in promiscuous mode (by design). This is intentional
> because a bad frame is just that, bad, and with no hope of knowing if the
> data in it is okay/malicious/other. I understand your reasoning above,
> but realistically the rx_save_bad_frames should NOT be set. I'd ack a
> patch to comment that line out.
>
>> This helped me a lot to debug an FPGA ethernet core.
>> Maybe it can be also useful to someone else..
>
> I think this patch is just that, debug only. As a developer I understand
> why this is useful, but there is no reason any normal user would be able
> to benefit from this, so for now, sorry:
>
> NACK.
I think anyone sniffing a funky network would have benefit in
receiving all frames. So, while it shouldn't be enabled by default,
it would be nice to have an ethtool command to turn on receiving
bad-crc frames, as well as receiving the 4-byte CRC on the end of
the packets.
It just so happens I have such a patch, in case others agree :)
Thanks,
Ben
>
>>
>> Thanks
>> Andrea
>>
>> --- drivers/net/e100_orig.c 2011-06-14 23:29:38.322267075 +0200
>> +++ drivers/net/e100.c 2011-06-14 23:34:10.700791472 +0200
>> @@ -1975,7 +1975,8 @@ static int e100_rx_indicate(struct nic *
>> skb_put(skb, actual_size);
>> skb->protocol = eth_type_trans(skb, nic->netdev);
>>
>> - if (unlikely(!(rfd_status& cb_ok))) {
>> + if (unlikely(!(nic->flags& promiscuous || nic->loopback)&&
>> + !(rfd_status& cb_ok))) {
>> /* Don't indicate if hardware indicates errors */
>> dev_kfree_skb_any(skb);
>> } else if (actual_size> ETH_DATA_LEN + VLAN_ETH_HLEN) {
>>
> --
> 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
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Discover what all the cheering's about.
Get your free trial download today.
http://p.sf.net/sfu/quest-dev2dev2
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [E1000-devel] [PATCH] e100: Fix inconsistency in bad frames handling
2011-06-06 17:56 ` Ben Greear
@ 2011-06-06 20:15 ` Ben Hutchings
2011-06-06 20:20 ` Ben Greear
2011-06-06 20:29 ` Eric Dumazet
0 siblings, 2 replies; 9+ messages in thread
From: Ben Hutchings @ 2011-06-06 20:15 UTC (permalink / raw)
To: Ben Greear
Cc: Brandeburg, Jesse, Andrea Merello,
e1000-devel@lists.sourceforge.net, netdev
On Mon, 2011-06-06 at 10:56 -0700, Ben Greear wrote:
> On 06/06/2011 10:49 AM, Brandeburg, Jesse wrote:
> >
> > <added netdev>, removed other useless lists.
> >
> > On Sat, 4 Jun 2011, Andrea Merello wrote:
> >> In e100 driver it seems that the intention was to accept bad frames in
> >> promiscuous mode and loopback mode.
> >> I think this is evident because of the following code in the driver:
> >>
> >> if (nic->flags& promiscuous || nic->loopback) {
> >> config->rx_save_bad_frames = 0x1; /* 1=save, 0=discard */
> >> config->rx_discard_short_frames = 0x0; /* 1=discard, 0=save */
> >> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
> >> }
> >>
> >
> > Hi, thanks for your work on e100.
> >
> >> However this intention is not really realized because bad frames are
> >> discarded later by SW check.
> >> This patch finally honors the above intention, making the RX code to
> >> let bad frames to pass when the NIC is in promiscuous or loopback
> >> mode.
> >
> > I think this may be a mistake by the authors of the software developers
> > manual. The manual suggests that save bad frames and save short frames
> > should be enabled in promisc mode, but all of our other drivers *do not*
> > save bad frames when in promiscuous mode (by design). This is intentional
> > because a bad frame is just that, bad, and with no hope of knowing if the
> > data in it is okay/malicious/other. I understand your reasoning above,
> > but realistically the rx_save_bad_frames should NOT be set. I'd ack a
> > patch to comment that line out.
> >
> >> This helped me a lot to debug an FPGA ethernet core.
> >> Maybe it can be also useful to someone else..
> >
> > I think this patch is just that, debug only. As a developer I understand
> > why this is useful, but there is no reason any normal user would be able
> > to benefit from this, so for now, sorry:
> >
> > NACK.
>
> I think anyone sniffing a funky network would have benefit in
> receiving all frames. So, while it shouldn't be enabled by default,
> it would be nice to have an ethtool command to turn on receiving
> bad-crc frames, as well as receiving the 4-byte CRC on the end of
> the packets.
>
> It just so happens I have such a patch, in case others agree :)
How would a received skb be flagged as having a CRC error?
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] e100: Fix inconsistency in bad frames handling
2011-06-06 20:15 ` [E1000-devel] " Ben Hutchings
@ 2011-06-06 20:20 ` Ben Greear
2011-06-06 20:29 ` Eric Dumazet
1 sibling, 0 replies; 9+ messages in thread
From: Ben Greear @ 2011-06-06 20:20 UTC (permalink / raw)
To: Ben Hutchings
Cc: Andrea Merello, e1000-devel@lists.sourceforge.net,
Brandeburg, Jesse, netdev
On 06/06/2011 01:15 PM, Ben Hutchings wrote:
> On Mon, 2011-06-06 at 10:56 -0700, Ben Greear wrote:
>> On 06/06/2011 10:49 AM, Brandeburg, Jesse wrote:
>>>
>>> <added netdev>, removed other useless lists.
>>>
>>> On Sat, 4 Jun 2011, Andrea Merello wrote:
>>>> In e100 driver it seems that the intention was to accept bad frames in
>>>> promiscuous mode and loopback mode.
>>>> I think this is evident because of the following code in the driver:
>>>>
>>>> if (nic->flags& promiscuous || nic->loopback) {
>>>> config->rx_save_bad_frames = 0x1; /* 1=save, 0=discard */
>>>> config->rx_discard_short_frames = 0x0; /* 1=discard, 0=save */
>>>> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
>>>> }
>>>>
>>>
>>> Hi, thanks for your work on e100.
>>>
>>>> However this intention is not really realized because bad frames are
>>>> discarded later by SW check.
>>>> This patch finally honors the above intention, making the RX code to
>>>> let bad frames to pass when the NIC is in promiscuous or loopback
>>>> mode.
>>>
>>> I think this may be a mistake by the authors of the software developers
>>> manual. The manual suggests that save bad frames and save short frames
>>> should be enabled in promisc mode, but all of our other drivers *do not*
>>> save bad frames when in promiscuous mode (by design). This is intentional
>>> because a bad frame is just that, bad, and with no hope of knowing if the
>>> data in it is okay/malicious/other. I understand your reasoning above,
>>> but realistically the rx_save_bad_frames should NOT be set. I'd ack a
>>> patch to comment that line out.
>>>
>>>> This helped me a lot to debug an FPGA ethernet core.
>>>> Maybe it can be also useful to someone else..
>>>
>>> I think this patch is just that, debug only. As a developer I understand
>>> why this is useful, but there is no reason any normal user would be able
>>> to benefit from this, so for now, sorry:
>>>
>>> NACK.
>>
>> I think anyone sniffing a funky network would have benefit in
>> receiving all frames. So, while it shouldn't be enabled by default,
>> it would be nice to have an ethtool command to turn on receiving
>> bad-crc frames, as well as receiving the 4-byte CRC on the end of
>> the packets.
>>
>> It just so happens I have such a patch, in case others agree :)
>
> How would a received skb be flagged as having a CRC error?
We could add an skb flag? Or just pass it straight up and let
the receiving code deal with it (if we pass up the CRC as well, receiving
code has all info needed).
There are other errors possible as well, I think...maybe short frames, etc,
so it might be nice to have some way to mark what the NIC thinks is wrong
with the pkt.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Discover what all the cheering's about.
Get your free trial download today.
http://p.sf.net/sfu/quest-dev2dev2
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] e100: Fix inconsistency in bad frames handling
2011-06-06 20:15 ` [E1000-devel] " Ben Hutchings
2011-06-06 20:20 ` Ben Greear
@ 2011-06-06 20:29 ` Eric Dumazet
2011-06-06 22:45 ` Ben Greear
2011-06-14 17:30 ` Ben Greear
1 sibling, 2 replies; 9+ messages in thread
From: Eric Dumazet @ 2011-06-06 20:29 UTC (permalink / raw)
To: Ben Hutchings
Cc: Andrea Merello, e1000-devel@lists.sourceforge.net,
Brandeburg, Jesse, netdev
Le lundi 06 juin 2011 à 21:15 +0100, Ben Hutchings a écrit :
> On Mon, 2011-06-06 at 10:56 -0700, Ben Greear wrote:
> > On 06/06/2011 10:49 AM, Brandeburg, Jesse wrote:
> > >
> > > <added netdev>, removed other useless lists.
> > >
> > > On Sat, 4 Jun 2011, Andrea Merello wrote:
> > >> In e100 driver it seems that the intention was to accept bad frames in
> > >> promiscuous mode and loopback mode.
> > >> I think this is evident because of the following code in the driver:
> > >>
> > >> if (nic->flags& promiscuous || nic->loopback) {
> > >> config->rx_save_bad_frames = 0x1; /* 1=save, 0=discard */
> > >> config->rx_discard_short_frames = 0x0; /* 1=discard, 0=save */
> > >> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
> > >> }
> > >>
> > >
> > > Hi, thanks for your work on e100.
> > >
> > >> However this intention is not really realized because bad frames are
> > >> discarded later by SW check.
> > >> This patch finally honors the above intention, making the RX code to
> > >> let bad frames to pass when the NIC is in promiscuous or loopback
> > >> mode.
> > >
> > > I think this may be a mistake by the authors of the software developers
> > > manual. The manual suggests that save bad frames and save short frames
> > > should be enabled in promisc mode, but all of our other drivers *do not*
> > > save bad frames when in promiscuous mode (by design). This is intentional
> > > because a bad frame is just that, bad, and with no hope of knowing if the
> > > data in it is okay/malicious/other. I understand your reasoning above,
> > > but realistically the rx_save_bad_frames should NOT be set. I'd ack a
> > > patch to comment that line out.
> > >
> > >> This helped me a lot to debug an FPGA ethernet core.
> > >> Maybe it can be also useful to someone else..
> > >
> > > I think this patch is just that, debug only. As a developer I understand
> > > why this is useful, but there is no reason any normal user would be able
> > > to benefit from this, so for now, sorry:
> > >
> > > NACK.
> >
> > I think anyone sniffing a funky network would have benefit in
> > receiving all frames. So, while it shouldn't be enabled by default,
> > it would be nice to have an ethtool command to turn on receiving
> > bad-crc frames, as well as receiving the 4-byte CRC on the end of
> > the packets.
> >
> > It just so happens I have such a patch, in case others agree :)
>
> How would a received skb be flagged as having a CRC error?
>
maybe some skb->pkt_type = PACKET_INVALID; or something...
------------------------------------------------------------------------------
Simplify data backup and recovery for your virtual environment with vRanger.
Installation's a snap, and flexible recovery options mean your data is safe,
secure and there when you need it. Discover what all the cheering's about.
Get your free trial download today.
http://p.sf.net/sfu/quest-dev2dev2
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] e100: Fix inconsistency in bad frames handling
2011-06-06 20:29 ` Eric Dumazet
@ 2011-06-06 22:45 ` Ben Greear
2011-06-14 17:30 ` Ben Greear
1 sibling, 0 replies; 9+ messages in thread
From: Ben Greear @ 2011-06-06 22:45 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, Andrea Merello, netdev, Brandeburg, Jesse,
e1000-devel@lists.sourceforge.net
On 06/06/2011 01:29 PM, Eric Dumazet wrote:
> Le lundi 06 juin 2011 à 21:15 +0100, Ben Hutchings a écrit :
>> On Mon, 2011-06-06 at 10:56 -0700, Ben Greear wrote:
>>> On 06/06/2011 10:49 AM, Brandeburg, Jesse wrote:
>>>>
>>>> <added netdev>, removed other useless lists.
>>>>
>>>> On Sat, 4 Jun 2011, Andrea Merello wrote:
>>>>> In e100 driver it seems that the intention was to accept bad frames in
>>>>> promiscuous mode and loopback mode.
>>>>> I think this is evident because of the following code in the driver:
>>>>>
>>>>> if (nic->flags& promiscuous || nic->loopback) {
>>>>> config->rx_save_bad_frames = 0x1; /* 1=save, 0=discard */
>>>>> config->rx_discard_short_frames = 0x0; /* 1=discard, 0=save */
>>>>> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
>>>>> }
>>>>>
>>>>
>>>> Hi, thanks for your work on e100.
>>>>
>>>>> However this intention is not really realized because bad frames are
>>>>> discarded later by SW check.
>>>>> This patch finally honors the above intention, making the RX code to
>>>>> let bad frames to pass when the NIC is in promiscuous or loopback
>>>>> mode.
>>>>
>>>> I think this may be a mistake by the authors of the software developers
>>>> manual. The manual suggests that save bad frames and save short frames
>>>> should be enabled in promisc mode, but all of our other drivers *do not*
>>>> save bad frames when in promiscuous mode (by design). This is intentional
>>>> because a bad frame is just that, bad, and with no hope of knowing if the
>>>> data in it is okay/malicious/other. I understand your reasoning above,
>>>> but realistically the rx_save_bad_frames should NOT be set. I'd ack a
>>>> patch to comment that line out.
>>>>
>>>>> This helped me a lot to debug an FPGA ethernet core.
>>>>> Maybe it can be also useful to someone else..
>>>>
>>>> I think this patch is just that, debug only. As a developer I understand
>>>> why this is useful, but there is no reason any normal user would be able
>>>> to benefit from this, so for now, sorry:
>>>>
>>>> NACK.
>>>
>>> I think anyone sniffing a funky network would have benefit in
>>> receiving all frames. So, while it shouldn't be enabled by default,
>>> it would be nice to have an ethtool command to turn on receiving
>>> bad-crc frames, as well as receiving the 4-byte CRC on the end of
>>> the packets.
>>>
>>> It just so happens I have such a patch, in case others agree :)
>>
>> How would a received skb be flagged as having a CRC error?
>>
>
> maybe some skb->pkt_type = PACKET_INVALID; or something...
That looks good to me. pkt_type is passed up through some of the pf_socket
interfaces, so capture tools could easily be modified to pay attention to it.
We might also need to add a flag 'crc-included' so that tools could know
that the last 4 bytes of the packet are ethernet CRC, for NICs that support
that.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] e100: Fix inconsistency in bad frames handling
2011-06-06 20:29 ` Eric Dumazet
2011-06-06 22:45 ` Ben Greear
@ 2011-06-14 17:30 ` Ben Greear
2011-06-14 18:57 ` [E1000-devel] " Brandeburg, Jesse
1 sibling, 1 reply; 9+ messages in thread
From: Ben Greear @ 2011-06-14 17:30 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ben Hutchings, Andrea Merello, netdev, Brandeburg, Jesse,
e1000-devel@lists.sourceforge.net
On 06/06/2011 01:29 PM, Eric Dumazet wrote:
> Le lundi 06 juin 2011 à 21:15 +0100, Ben Hutchings a écrit :
>> On Mon, 2011-06-06 at 10:56 -0700, Ben Greear wrote:
>>> On 06/06/2011 10:49 AM, Brandeburg, Jesse wrote:
>>>>
>>>> <added netdev>, removed other useless lists.
>>>>
>>>> On Sat, 4 Jun 2011, Andrea Merello wrote:
>>>>> In e100 driver it seems that the intention was to accept bad frames in
>>>>> promiscuous mode and loopback mode.
>>>>> I think this is evident because of the following code in the driver:
>>>>>
>>>>> if (nic->flags& promiscuous || nic->loopback) {
>>>>> config->rx_save_bad_frames = 0x1; /* 1=save, 0=discard */
>>>>> config->rx_discard_short_frames = 0x0; /* 1=discard, 0=save */
>>>>> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
>>>>> }
>>>>>
>>>>
>>>> Hi, thanks for your work on e100.
>>>>
>>>>> However this intention is not really realized because bad frames are
>>>>> discarded later by SW check.
>>>>> This patch finally honors the above intention, making the RX code to
>>>>> let bad frames to pass when the NIC is in promiscuous or loopback
>>>>> mode.
>>>>
>>>> I think this may be a mistake by the authors of the software developers
>>>> manual. The manual suggests that save bad frames and save short frames
>>>> should be enabled in promisc mode, but all of our other drivers *do not*
>>>> save bad frames when in promiscuous mode (by design). This is intentional
>>>> because a bad frame is just that, bad, and with no hope of knowing if the
>>>> data in it is okay/malicious/other. I understand your reasoning above,
>>>> but realistically the rx_save_bad_frames should NOT be set. I'd ack a
>>>> patch to comment that line out.
>>>>
>>>>> This helped me a lot to debug an FPGA ethernet core.
>>>>> Maybe it can be also useful to someone else..
>>>>
>>>> I think this patch is just that, debug only. As a developer I understand
>>>> why this is useful, but there is no reason any normal user would be able
>>>> to benefit from this, so for now, sorry:
>>>>
>>>> NACK.
>>>
>>> I think anyone sniffing a funky network would have benefit in
>>> receiving all frames. So, while it shouldn't be enabled by default,
>>> it would be nice to have an ethtool command to turn on receiving
>>> bad-crc frames, as well as receiving the 4-byte CRC on the end of
>>> the packets.
>>>
>>> It just so happens I have such a patch, in case others agree :)
>>
>> How would a received skb be flagged as having a CRC error?
>>
>
> maybe some skb->pkt_type = PACKET_INVALID; or something...
Jesse: If I can get the ethtool related patches accepted, would
you accept patches to e100 (and other Intel drivers) for
this feature?
Thanks,
Ben
>
>
>
> --
> 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
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [E1000-devel] [PATCH] e100: Fix inconsistency in bad frames handling
2011-06-14 17:30 ` Ben Greear
@ 2011-06-14 18:57 ` Brandeburg, Jesse
2011-06-14 19:05 ` Ben Greear
0 siblings, 1 reply; 9+ messages in thread
From: Brandeburg, Jesse @ 2011-06-14 18:57 UTC (permalink / raw)
To: Ben Greear
Cc: Eric Dumazet, Ben Hutchings, Andrea Merello,
e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
On Tue, 2011-06-14 at 10:30 -0700, Ben Greear wrote:
> >> How would a received skb be flagged as having a CRC error?
> >>
> >
> > maybe some skb->pkt_type = PACKET_INVALID; or something...
>
> Jesse: If I can get the ethtool related patches accepted, would
> you accept patches to e100 (and other Intel drivers) for
> this feature?
seems like a reasonable thing, but, there is some risk that might
prevent us turning this on however, because we often like our hardware
to discard bad frames because (especially) long ones can use quite a few
buffers.
I still am generally uncomfortable with this idea. We've survived a
long time without it and it opens up the possibility of extra bugs (like
possible security issues, etc) with very little opportunity for
worthwhile gain.
Jesse
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [E1000-devel] [PATCH] e100: Fix inconsistency in bad frames handling
2011-06-14 18:57 ` [E1000-devel] " Brandeburg, Jesse
@ 2011-06-14 19:05 ` Ben Greear
0 siblings, 0 replies; 9+ messages in thread
From: Ben Greear @ 2011-06-14 19:05 UTC (permalink / raw)
To: Brandeburg, Jesse
Cc: Eric Dumazet, Ben Hutchings, Andrea Merello,
e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
On 06/14/2011 11:57 AM, Brandeburg, Jesse wrote:
> On Tue, 2011-06-14 at 10:30 -0700, Ben Greear wrote:
>>>> How would a received skb be flagged as having a CRC error?
>>>>
>>>
>>> maybe some skb->pkt_type = PACKET_INVALID; or something...
>>
>> Jesse: If I can get the ethtool related patches accepted, would
>> you accept patches to e100 (and other Intel drivers) for
>> this feature?
>
> seems like a reasonable thing, but, there is some risk that might
> prevent us turning this on however, because we often like our hardware
> to discard bad frames because (especially) long ones can use quite a few
> buffers.
>
> I still am generally uncomfortable with this idea. We've survived a
> long time without it and it opens up the possibility of extra bugs (like
> possible security issues, etc) with very little opportunity for
> worthwhile gain.
If we make it require root permissions to enable this, and disable
it by default, will that be enough allay your fears?
Also, could print msg to kernel logs when enabling this.
I think that we could skip any intrusive changes (ie, if allowing
rx of long packets is a big problem, just don't allow that for
that particular driver.)
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-06-14 19:05 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <BANLkTimpYniHhN6ccMXN7Lx3xDdK6sC+FQ@mail.gmail.com>
2011-06-06 17:49 ` [E1000-devel] [PATCH] e100: Fix inconsistency in bad frames handling Brandeburg, Jesse
2011-06-06 17:56 ` Ben Greear
2011-06-06 20:15 ` [E1000-devel] " Ben Hutchings
2011-06-06 20:20 ` Ben Greear
2011-06-06 20:29 ` Eric Dumazet
2011-06-06 22:45 ` Ben Greear
2011-06-14 17:30 ` Ben Greear
2011-06-14 18:57 ` [E1000-devel] " Brandeburg, Jesse
2011-06-14 19:05 ` Ben Greear
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).