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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 06 Mar 2014 11:33:29 -0800
From:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To:	Shannon Nelson <shannon.nelson@...el.com>
Cc:	Joe Perches <joe@...ches.com>, davem@...emloft.net,
	netdev@...r.kernel.org, gospo@...hat.com, sassmann@...hat.com,
	Kavindya Deegala <kavindya.s.deegala@...el.com>
Subject: Re: [net-next 01/13] i40e: use ether_addr_equal_64bits

On Thu, 2014-03-06 at 10:10 -0800, Shannon Nelson wrote:
> No, none of those uses are fast path.  I'd prefer we drop this patch.
> sln
> 

Ok based on Joe's and Shannon's responses, I will drop this patch from
the series.

Dave- v2 of the series will be coming shortly.

> 
> On Wed, Mar 5, 2014 at 8:40 PM, Jeff Kirsher
> <jeffrey.t.kirsher@...el.com> wrote:
>         On Wed, 2014-03-05 at 20:35 -0800, Joe Perches wrote:
>         > On Wed, 2014-03-05 at 20:21 -0800, Jeff Kirsher wrote:
>         > > From: Joe Perches <joe@...ches.com>
>         > >
>         > > All ether_addr_equal tests in i40e can use the
>         > > slightly more efficient ether_addr_equal_64bits.
>         > >
>         > > All addresses passed to the various functions that
>         > > use ether_addr_equal are using structs that have 2
>         > > or more bytes of additional data after the mac addr
>         > > being tested.
>         >
>         > Hey Jeff.  Hi Kavindya.
>         >
>         > I don't know if these are really that important to convert.
>         >
>         > I did these when I looked for an example to convert and
>         > I thought "hey, 40 gig's fast, I should start there..."
>         >
>         > Recently, I've come to think that ether_addr_equal should
>         > be used almost exclusively and ether_addr_equal_64bits
>         > should only be used on performance sensitive or fast-path
>         > code.
>         >
>         > Are any of these on anything like a fast path or are
>         > performance sensitive?
>         
>         
>         Not sure if Kavindya has before and after performance numbers,
>         hopefully
>         she does and can provide the numbers tomorrow.
>         
>         >
>         > If not, maybe this patch should get tossed.
>         >
>         > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
>         b/drivers/net/ethernet/intel/i40e/i40e_main.c
>         > []
>         > > @@ -997,7 +997,7 @@ static struct i40e_mac_filter
>         *i40e_find_filter(struct i40e_vsi *vsi,
>         > >             return NULL;
>         > >
>         > >     list_for_each_entry(f, &vsi->mac_filter_list, list) {
>         > > -           if ((ether_addr_equal(macaddr, f->macaddr)) &&
>         > > +           if (ether_addr_equal_64bits(macaddr,
>         f->macaddr) &&
>         > >                 (vlan == f->vlan)    &&
>         > >                 (!is_vf || f->is_vf) &&
>         > >                 (!is_netdev || f->is_netdev))
>         > > @@ -1025,7 +1025,7 @@ struct i40e_mac_filter
>         *i40e_find_mac(struct i40e_vsi *vsi, u8 *macaddr,
>         > >             return NULL;
>         > >
>         > >     list_for_each_entry(f, &vsi->mac_filter_list, list) {
>         > > -           if ((ether_addr_equal(macaddr, f->macaddr)) &&
>         > > +           if (ether_addr_equal_64bits(macaddr,
>         f->macaddr) &&
>         > >                 (!is_vf || f->is_vf) &&
>         > >                 (!is_netdev || f->is_netdev))
>         > >                     return f;
>         >
>         > Well, maybe these 2.
>         >
>         > > @@ -1214,7 +1214,7 @@ static int i40e_set_mac(struct
>         net_device *netdev, void *p)
>         > >
>         > >     netdev_info(netdev, "set mac address=%pM\n",
>         addr->sa_data);
>         > >
>         > > -   if (ether_addr_equal(netdev->dev_addr, addr->sa_data))
>         > > +   if (ether_addr_equal_64bits(netdev->dev_addr,
>         addr->sa_data))
>         > >             return 0;
>         > >
>         > >     if (test_bit(__I40E_DOWN, &vsi->back->state) ||
>         >
>         > probably not.
>         >
>         > > @@ -1417,21 +1417,24 @@ static void
>         i40e_set_rx_mode(struct net_device *netdev)
>         > >
>         > >             if (is_multicast_ether_addr(f->macaddr)) {
>         > >                     netdev_for_each_mc_addr(mca, netdev) {
>         > > -                           if
>         (ether_addr_equal(mca->addr, f->macaddr)) {
>         > > +                           if
>         (ether_addr_equal_64bits(mca->addr,
>         > > +
>         f->macaddr)) {
>         > >                                     found = true;
>         > >                                     break;
>         > >                             }
>         > >                     }
>         > >             } else {
>         > >                     netdev_for_each_uc_addr(uca, netdev) {
>         > > -                           if
>         (ether_addr_equal(uca->addr, f->macaddr)) {
>         > > +                           if
>         (ether_addr_equal_64bits(uca->addr,
>         > > +
>         f->macaddr)) {
>         > >                                     found = true;
>         > >                                     break;
>         > >                             }
>         > >                     }
>         > >
>         > >                     for_each_dev_addr(netdev, ha) {
>         > > -                           if (ether_addr_equal(ha->addr,
>         f->macaddr)) {
>         > > +                           if
>         (ether_addr_equal_64bits(ha->addr,
>         > > +
>         f->macaddr)) {
>         > >                                     found = true;
>         > >                                     break;
>         > >                             }
>         >
>         > probably not.
>         >
>         > > @@ -1905,7 +1908,8 @@ int i40e_vsi_kill_vlan(struct
>         i40e_vsi *vsi, s16 vid)
>         > >     list_for_each_entry(f, &vsi->mac_filter_list, list) {
>         > >             if (is_netdev) {
>         > >                     if (f->vlan &&
>         > > -                       ether_addr_equal(netdev->dev_addr,
>         f->macaddr))
>         > > +
>         ether_addr_equal_64bits(netdev->dev_addr,
>         > > +
>         f->macaddr))
>         > >                             filter_count++;
>         > >             }
>         >
>         > Dunno.
>         >
>         
>         
> 
> 
> 
> 
> -- 
> ==============================================
> Mr. Shannon Nelson         Parents can't afford to be squeamish.



Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ