On 12/19/2013 10:08 AM, Florian Fainelli wrote:
> Hi John,
>
> In commit 815cccbf ("ixgbe: add setlink, getlink support to ixgbe and
> ixgbevf") this line was added:
>
>
> + if ((skb->pkt_type & (PACKET_BROADCAST | PACKET_MULTICAST)) &&
> + !(compare_ether_addr(adapter->netdev->dev_addr,
> + eth_hdr(skb)->h_source))) {
>
> This looks suspicious to me since skb->pkt_type is not a bitmask, but
> holds only one value at a time, and with this check you would also
> match any value which has the lower two bits set, so PACKET_OTHERHOST,
> PACKET_LOOPBACK and PACKET_FASTROUTE are also matching the first part
> of the check.
>
Yep, it is poorly coded I'll send a fix to Jeff to make it readable.
Note it doesn't actually break anything in practice because the only
types that can be set at this point are broadcast, multicast or
otherhost. And because this virtual function is behind the embedded
switch in the nic only packets with the correct destination address
or multicast addresses will make it to the virtual function. The
virtual function doesn't currently support promisc mode further promisc
mode in the context of a switch that doesn't support flooding is sort
of non-sense to start with. All that said a much better check would
be 'if (skb->pkt_type && ...) ' thanks for catching it.
Thanks,
John
------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT
organizations don't have a clear picture of how application performance
affects their revenue. With AppDynamics, you get 100% visibility into your
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit
http://communities.intel.com/community/wired
