[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20120321162553.GA11165@quad.redhat.com>
Date: Wed, 21 Mar 2012 12:25:53 -0400
From: Andy Gospodarek <gospo@...hat.com>
To: David Miller <davem@...emloft.net>
Cc: andy@...yhouse.net, fubar@...ibm.com, netdev@...r.kernel.org,
ralf.zeidler@....com
Subject: Re: [PATCH net-next] bonding: remove entries for master_ip and
vlan_ip and query devices instead
On Fri, Mar 16, 2012 at 10:55:33PM -0700, David Miller wrote:
> From: Andy Gospodarek <andy@...yhouse.net>
> Date: Fri, 16 Mar 2012 09:48:23 -0400
>
> > On Thu, Mar 15, 2012 at 9:03 PM, Jay Vosburgh <fubar@...ibm.com> wrote:
> >>>@@ -2618,7 +2624,9 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> >>> if (!bond_vlan_used(bond)) {
> >>> pr_debug("basa: empty vlan: arp_send\n");
> >>> bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
> >>>- bond->master_ip, 0);
> >>>+ bond_confirm_addr(bond->dev,
> >>>+ targets[i],
> >>>+ 0), 0);
> >>
> >> Same comment here and for the later calls to bond_confirm_addr,
> >> here putting "targets[i]" and perhaps the 0 on the previous line,
> >> although I'm less sure that it won't look funky.
> >>
> >> -J
> >>
> >
> > These we a bit tough to get looking great. What I did really seemed
> > like the best I could do and keep it to a reasonable length. If you
> > want me to just keep the length of these lines <100 characters wide, I
> > could combine them into the same line. Either way is fine with me,
> > but I really just didn't want the code to get too wide and hard to
> > read when using a standard size terminal.
>
> It seems to me that the easiest thing to do is:
>
> __be32 addr = bond_confirm_addr(bond->dev, targets[i], 0);
> bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i], addr, 0);
>
> And actually this sequence is used in three places, so even better
> to put it into a helper function.
For readability it makes sense to pop this function out like you have
suggested. I'm not sure I want to make a helper function for both
calls, but I'll take a look at post an update patch.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists