* [PATCH net-next] ixgbevf: fix skb->pkt_type checks
@ 2014-02-26 22:44 Florian Fainelli
2014-02-27 21:07 ` David Miller
2014-02-27 22:58 ` Aaron Brown
0 siblings, 2 replies; 6+ messages in thread
From: Florian Fainelli @ 2014-02-26 22:44 UTC (permalink / raw)
To: netdev
Cc: e1000-devel, aaron.f.brown, alexander.h.duyck, jesse.brandeburg,
Florian Fainelli
skb->pkt_type is not a bitmask, but contains only value at a time from
the range defined in include/uapi/linux/if_packet.h.
Checking it like if it was a bitmask of values would also cause
PACKET_OTHERHOST, PACKET_LOOPBACK and PACKET_FASTROUTE to be matched by this
check since their lower 2 bits are also set, although that does not fix a real
bug due to how the checks are used, it is still potentially confusing.
This bogus check was introduced in commit 815cccbf ("ixgbe: add setlink,
getlink support to ixgbe and ixgbevf").
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index a6af7b7..c6be010 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -516,8 +516,9 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
/* Workaround hardware that can't do proper VEPA multicast
* source pruning.
*/
- if ((skb->pkt_type & (PACKET_BROADCAST | PACKET_MULTICAST)) &&
- ether_addr_equal(rx_ring->netdev->dev_addr,
+ if ((skb->pkt_type == PACKET_BROADCAST ||
+ skb->pkt_type == PACKET_MULTICAST) &&
+ ether_addr_equal(adapter->netdev->dev_addr,
eth_hdr(skb)->h_source)) {
dev_kfree_skb_irq(skb);
goto next_desc;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ixgbevf: fix skb->pkt_type checks
2014-02-26 22:44 [PATCH net-next] ixgbevf: fix skb->pkt_type checks Florian Fainelli
@ 2014-02-27 21:07 ` David Miller
2014-02-27 22:39 ` Skidmore, Donald C
2014-02-27 22:58 ` Aaron Brown
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2014-02-27 21:07 UTC (permalink / raw)
To: f.fainelli
Cc: netdev, e1000-devel, aaron.f.brown, alexander.h.duyck, jesse.brandeburg
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed, 26 Feb 2014 14:44:00 -0800
> skb->pkt_type is not a bitmask, but contains only value at a time from
> the range defined in include/uapi/linux/if_packet.h.
>
> Checking it like if it was a bitmask of values would also cause
> PACKET_OTHERHOST, PACKET_LOOPBACK and PACKET_FASTROUTE to be matched by this
> check since their lower 2 bits are also set, although that does not fix a real
> bug due to how the checks are used, it is still potentially confusing.
>
> This bogus check was introduced in commit 815cccbf ("ixgbe: add setlink,
> getlink support to ixgbe and ixgbevf").
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Intel folks, I assume you will pick this up?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ixgbevf: fix skb->pkt_type checks
2014-02-27 21:07 ` David Miller
@ 2014-02-27 22:39 ` Skidmore, Donald C
0 siblings, 0 replies; 6+ messages in thread
From: Skidmore, Donald C @ 2014-02-27 22:39 UTC (permalink / raw)
To: David Miller, f.fainelli; +Cc: e1000-devel, netdev, Brandeburg, Jesse
> -----Original Message-----
> From: David Miller [mailto:davem@redhat.com]
> Sent: Thursday, February 27, 2014 1:08 PM
> To: f.fainelli@gmail.com
> Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org;
> Brandeburg, Jesse
> Subject: Re: [E1000-devel] [PATCH net-next] ixgbevf: fix skb->pkt_type
> checks
>
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Wed, 26 Feb 2014 14:44:00 -0800
>
> > skb->pkt_type is not a bitmask, but contains only value at a time from
> > the range defined in include/uapi/linux/if_packet.h.
> >
> > Checking it like if it was a bitmask of values would also cause
> > PACKET_OTHERHOST, PACKET_LOOPBACK and PACKET_FASTROUTE to be
> matched
> > by this check since their lower 2 bits are also set, although that
> > does not fix a real bug due to how the checks are used, it is still potentially
> confusing.
> >
> > This bogus check was introduced in commit 815cccbf ("ixgbe: add
> > setlink, getlink support to ixgbe and ixgbevf").
> >
> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> Intel folks, I assume you will pick this up?
>
This patch looks like it fixes an obvious bug on our part. I'm sure Aaron will get to it soon.
Thanks Florin for pointing this out,
-Don Skidmore <donald.c.skidmore@intel.com>
------------------------------------------------------------------------------
Flow-based real-time traffic analytics software. Cisco certified tool.
Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
Customize your own dashboards, set traffic alerts and generate reports.
Network behavioral analysis & security monitoring. All-in-one tool.
http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk
_______________________________________________
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] 6+ messages in thread
* Re: [PATCH net-next] ixgbevf: fix skb->pkt_type checks
2014-02-26 22:44 [PATCH net-next] ixgbevf: fix skb->pkt_type checks Florian Fainelli
2014-02-27 21:07 ` David Miller
@ 2014-02-27 22:58 ` Aaron Brown
2014-02-28 23:26 ` Brown, Aaron F
1 sibling, 1 reply; 6+ messages in thread
From: Aaron Brown @ 2014-02-27 22:58 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, e1000-devel, alexander.h.duyck, jesse.brandeburg
On Wed, 2014-02-26 at 14:44 -0800, Florian Fainelli wrote:
> skb->pkt_type is not a bitmask, but contains only value at a time from
> the range defined in include/uapi/linux/if_packet.h.
>
> Checking it like if it was a bitmask of values would also cause
> PACKET_OTHERHOST, PACKET_LOOPBACK and PACKET_FASTROUTE to be matched by this
> check since their lower 2 bits are also set, although that does not fix a real
> bug due to how the checks are used, it is still potentially confusing.
>
> This bogus check was introduced in commit 815cccbf ("ixgbe: add setlink,
> getlink support to ixgbe and ixgbevf").
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Thanks Florian, I have added this to our internal queue.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ixgbevf: fix skb->pkt_type checks
2014-02-27 22:58 ` Aaron Brown
@ 2014-02-28 23:26 ` Brown, Aaron F
2014-02-28 23:31 ` Florian Fainelli
0 siblings, 1 reply; 6+ messages in thread
From: Brown, Aaron F @ 2014-02-28 23:26 UTC (permalink / raw)
To: f.fainelli; +Cc: Duyck, Alexander H, Brandeburg, Jesse, netdev, e1000-devel
On Thu, 2014-02-27 at 14:58 -0800, Aaron Brown wrote:
> On Wed, 2014-02-26 at 14:44 -0800, Florian Fainelli wrote:
> > skb->pkt_type is not a bitmask, but contains only value at a time from
> > the range defined in include/uapi/linux/if_packet.h.
> >
> > Checking it like if it was a bitmask of values would also cause
> > PACKET_OTHERHOST, PACKET_LOOPBACK and PACKET_FASTROUTE to be matched by this
> > check since their lower 2 bits are also set, although that does not fix a real
> > bug due to how the checks are used, it is still potentially confusing.
> >
> > This bogus check was introduced in commit 815cccbf ("ixgbe: add setlink,
> > getlink support to ixgbe and ixgbevf").
> >
> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> > ---
> > drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
>
> Thanks Florian, I have added this to our internal queue.
Unfortunately this introduces a compile error:
----------------------------------------------
/usr/src/kbuild_net-next_community/arch/x86/Makefile:133: CONFIG_X86_X32
enabled but no binutils support
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c: In function
‘ixgbevf_clean_rx_irq’:
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:521: error: ‘adapter’
undeclared (first use in this function)
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:521: error: (Each
undeclared identifier is reported only once
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:521: error: for each
function it appears in.)
make[5]: *** [drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [drivers/net/ethernet/intel/ixgbevf] Error 2
make[3]: *** [drivers/net/ethernet/intel] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [drivers/net/ethernet] Error 2
make[1]: *** [drivers/net] Error 2
make: *** [drivers] Error 2
~
----------------------------------------------
Are willing to fix up a v2?
Thanks,
Aaron
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] ixgbevf: fix skb->pkt_type checks
2014-02-28 23:26 ` Brown, Aaron F
@ 2014-02-28 23:31 ` Florian Fainelli
0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2014-02-28 23:31 UTC (permalink / raw)
To: Brown, Aaron F; +Cc: Duyck, Alexander H, Brandeburg, Jesse, netdev, e1000-devel
2014-02-28 15:26 GMT-08:00 Brown, Aaron F <aaron.f.brown@intel.com>:
> On Thu, 2014-02-27 at 14:58 -0800, Aaron Brown wrote:
>> On Wed, 2014-02-26 at 14:44 -0800, Florian Fainelli wrote:
>> > skb->pkt_type is not a bitmask, but contains only value at a time from
>> > the range defined in include/uapi/linux/if_packet.h.
>> >
>> > Checking it like if it was a bitmask of values would also cause
>> > PACKET_OTHERHOST, PACKET_LOOPBACK and PACKET_FASTROUTE to be matched by this
>> > check since their lower 2 bits are also set, although that does not fix a real
>> > bug due to how the checks are used, it is still potentially confusing.
>> >
>> > This bogus check was introduced in commit 815cccbf ("ixgbe: add setlink,
>> > getlink support to ixgbe and ixgbevf").
>> >
>> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> > ---
>> > drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 5 +++--
>> > 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> Thanks Florian, I have added this to our internal queue.
>
> Unfortunately this introduces a compile error:
> ----------------------------------------------
> /usr/src/kbuild_net-next_community/arch/x86/Makefile:133: CONFIG_X86_X32
> enabled but no binutils support
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c: In function
> ‘ixgbevf_clean_rx_irq’:
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:521: error: ‘adapter’
> undeclared (first use in this function)
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:521: error: (Each
> undeclared identifier is reported only once
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c:521: error: for each
> function it appears in.)
> make[5]: *** [drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.o] Error 1
> make[5]: *** Waiting for unfinished jobs....
> make[4]: *** [drivers/net/ethernet/intel/ixgbevf] Error 2
> make[3]: *** [drivers/net/ethernet/intel] Error 2
> make[3]: *** Waiting for unfinished jobs....
> make[2]: *** [drivers/net/ethernet] Error 2
> make[1]: *** [drivers/net] Error 2
> make: *** [drivers] Error 2
> ~
> ----------------------------------------------
> Are willing to fix up a v2?
Sure, let me re-submit with a fixed version (seems like I rebased
against an earlier version).
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-28 23:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-26 22:44 [PATCH net-next] ixgbevf: fix skb->pkt_type checks Florian Fainelli
2014-02-27 21:07 ` David Miller
2014-02-27 22:39 ` Skidmore, Donald C
2014-02-27 22:58 ` Aaron Brown
2014-02-28 23:26 ` Brown, Aaron F
2014-02-28 23:31 ` Florian Fainelli
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.