All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarod Wilson <jarod@redhat.com>
To: David Miller <davem@davemloft.net>
Cc: linux-kernel@vger.kernel.org, j.vosburgh@gmail.com,
	vfalico@gmail.com, andy@greyhouse.net, netdev@vger.kernel.org
Subject: Re: [PATCH net-next] bonding: attempt to better support longer hw addresses
Date: Thu, 6 Apr 2017 14:01:06 -0400	[thread overview]
Message-ID: <fd21e0a4-0bd9-f07f-7204-2a95845dae38@redhat.com> (raw)
In-Reply-To: <20170405.184554.1608171595937690317.davem@davemloft.net>

On 2017-04-05 9:45 PM, David Miller wrote:
> From: Jarod Wilson <jarod@redhat.com>
> Date: Tue,  4 Apr 2017 17:32:42 -0400
...
> Applied, but:
> 
>> +static inline void bond_hw_addr_copy(u8 *dst, const u8 *src, unsigned int len)
>> +{
>> +	if (len == ETH_ALEN) {
>> +		ether_addr_copy(dst, src);
>> +		return;
>> +	}
>> +
>> +	memcpy(dst, src, len);
>> +}
> 
> I wonder how much value there is in trying to conditionally use
> ether_addr_copy().  Unless some of these calls are in the data
> plane, just a straight memcpy() all the time is fine and much
> simpler.

Yeah, I wasn't sure how much gain the bonding driver actually got from 
using the super-optimized ether_addr_copy(), and thought about just 
doing a memcpy all the time, but wanted to go for minimal impact to 
traditional ethernet bonding. Looks like bond_handle_frame() might 
benefit from sticking to ether_addr_copy() when it can, but the majority 
of other callers, at least in bond_main.c, are all in setup, teardown 
and failover paths, which ought to be tolerant of some overhead, though 
optimized failover isn't a bad thing for connection uptime. I do see 
bond_alb.c has one caller in the arp transmit path as well. I think I'm 
inclined to just leave well enough alone for the moment.

-- 
Jarod Wilson
jarod@redhat.com

      reply	other threads:[~2017-04-06 18:01 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-04 21:32 [PATCH net-next] bonding: attempt to better support longer hw addresses Jarod Wilson
2017-04-06  1:45 ` David Miller
2017-04-06 18:01   ` Jarod Wilson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fd21e0a4-0bd9-f07f-7204-2a95845dae38@redhat.com \
    --to=jarod@redhat.com \
    --cc=andy@greyhouse.net \
    --cc=davem@davemloft.net \
    --cc=j.vosburgh@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=vfalico@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.