t;> return of ad_agg_selection_logic).
>>
>> I believe I understand the described problem, but I don't see
>> how the patch fixes it. I suspect (but haven't tested) that the proper
>> fix is to acquire mode_lock in bond_update_slave_arr while ca
update_slave_arr remains false at
return of ad_agg_selection_logic).
I believe I understand the described problem, but I don't see
how the patch fixes it. I suspect (but haven't tested) that the proper
fix is to acquire mode_lock in bond_update_slave_arr while calling
bond_3ad_get_active_agg_info to avoid conflict with the state machine.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
address.
>+ * We'll make sure that slave no longer uses @slave's permanent address.
This is actually correct as written, but I can see that it's a
bit confusing. Rather than removing the second that, I'd suggest
changing it to "... make sure the other slave no longer uses ..." to be
clearer.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
The code in question was added by:
commit ee6377147409a00c071b2da853059a7d59979fbc
Author: Mahesh Bandewar
Date: Sat Oct 4 17:45:01 2014 -0700
bonding: Simplify the xmit function for modes that use xmit_hash
Mahesh, Nikolay, any thoughts?
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
uot;,
>+ opt->name, val->string);
>+ }
>+ break;
> default:
> break;
> }
>--
>2.25.1
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
for (port = aggregator->lag_ports; port;
>+ port = port->next_port_in_aggregator)
>+ if (port->aggregator == aggregator)
>+ port->aggregator = NULL;
> aggregator->lag_ports = NULL;
> aggregator->is_active = 0;
> aggregator->num_of_ports = 0;
>--
>2.23.0
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
ve been one patch, I suppose, but not
really a big deal. I'm in agreement about pr_err_once instead of
WARN_ONCE.
Acked-by: Jay Vosburgh
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
Provide documentation for src_valid_mark sysctl, which was added
in commit 28f6aeea3f12 ("net: restore ip source validation").
Signed-off-by: Jay Vosburgh
---
Documentation/networking/ip-sysctl.rst | 19 +++
1 file changed, 19 insertions(+)
diff --git a/Doc
tion logic has
failed to find an aggregator that matches, and also failed to find a
free aggregator.
I do need to fix up the failure handling here when it hits the
"did not find a suitable agg" case; the code here is simply wrong, and
has been wrong since the beginning. I'll hack the driver to induce this
situation rather than reproducing whatever problem is making it unable
to find a suitable aggregator.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
));
>>
>> analysis: port->aggregator is still NULL, which causes problem.
>>
>> aggregator = __get_first_agg(port);
>> ad_agg_selection_logic(aggregator, update_slave_arr);
>>
>> if (!port->aggregator->is_active)
>> port->actor_oper_port_state &= ~LACP_STATE_SYNCHRONIZATION;
Correct, if the "did not find a suitable aggregator" path is
taken, port->aggregator is NULL and bad things happen in the above
block.
This is something that needs to be fixed, but I'm also concerned
that there are other issues lurking, so I'd like to be able to reproduce
this.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
ted by copying bond_eth_hash(). :)
>
>> > +{
>> > + struct ethhdr *mac_hdr = (struct ethhdr *)skb_mac_header(skb);
>>
>> I don't see anything in the patch making sure the interface actually
>> has a L2 header. Should we validate somehow the ifc is Ethernet?
>
>I don't think it's necessary. There doesn't appear to be any explicit
>check for BOND_XMIT_POLICY_LAYER2 either. I believe we're guaranteed to
>not have anything but an ethernet header here, as the only other type I'm
>aware of being supported is Infiniband, but we limit that to active-backup
>only, and xmit_hash_policy isn't valid for active-backup.
This is correct, interfaces in a bond other than active-backup
will all be ARPHRD_ETHER. I'm unaware of a way to get a packet in there
without at least an Ethernet header.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
he number of slaves is
generally small (more than 2 is uncommon in my experience).
>> On the other hand, what's the worst that can happen if the GFP_ATOMIC
>> memory allocation fails. It's not like there is any data loss.
>> User space will retry when there is less me
Jarod Wilson wrote:
>On Thu, Jan 07, 2021 at 07:03:40PM -0500, Jarod Wilson wrote:
>> On Fri, Dec 18, 2020 at 04:18:59PM -0800, Jay Vosburgh wrote:
>> > Jarod Wilson wrote:
>> >
>> > >This comes from an end-user request, where they're running multip
x27;t go through the
bond.
The ARP monitor logic can only handle a limited set of
configurations, so if your configuration is outside of that it can
misbehave in some ways.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
oks sane to begin with, is
>fleshing out Documentation/networking/bonding.rst.
I'm sure you're aware, but any final submission will also need
to include netlink and iproute2 support.
>Cc: Jay Vosburgh
>Cc: Veaceslav Falico
>Cc: Andy Gospodarek
>Cc: "David
+;
>> }
>> }
>>
>> @@ -4117,6 +4118,7 @@ static struct slave *bond_get_slave_by_id(struct
>> bonding *bond,
>> break;
>> if (bond_slave_can_tx(slave))
>> return slave;
>> +bond->rr_tx_counter++;
>> }
>> /* no slave that can tx has been found */
>> return NULL;
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
gt;+ "active " : "backup ") : "",
>+ bond->params.downdelay *
>bond->params.miimon);
>+ }
>+ break;
>+
>+ case BOND_LINK_DOWN:
>+ slave_info(bond->dev, slave->dev, "link status down again after
>%d ms\n",
>+ (bond->params.updelay - slave->delay) *
>+ bond->params.miimon);
>+ break;
>+
>+ case BOND_LINK_BACK:
>+ if (slave->delay) {
>+ slave_info(bond->dev, slave->dev, "link status up,
>enabling it in %d ms\n",
>+ bond->params.updelay * bond->params.miimon);
>+ }
>+ break;
>+ }
>+
> if (notify) {
> bond_queue_slave_event(slave);
> bond_lower_state_changed(slave);
>--
>2.28.0
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
C (and others) have no idea what the dev is, it is not
>possible to ignore the event nor should it be ignored. It therefore seems
>the event shouldn't be sent for this situation. Please confirm the
>analysis above and provide a path forward since as currently implemented
>the functionality is broken.
As I said above, I don't think it's just about notifications.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
Tobias Waldekranz wrote:
>On Wed, Dec 02, 2020 at 11:09, Jay Vosburgh wrote:
>> Tobias Waldekranz wrote:
>>
>>>When creating a static bond (e.g. balance-xor), all ports will always
>>>be enabled. This is set, and the corresponding notification is sent
>>
Jarod Wilson wrote:
>On Wed, Dec 2, 2020 at 12:55 PM Jay Vosburgh
>wrote:
>>
>> Jarod Wilson wrote:
>>
>> >Don't try to adjust XFRM support flags if the bond device isn't yet
>> >registered. Bad things can currently happen when n
ond_lower_state_changed(new_slave);
>+
> res = bond_sysfs_slave_add(new_slave);
> if (res) {
> slave_dbg(bond_dev, slave_dev, "Error %d calling
> bond_sysfs_slave_add\n", res);
>--
>2.17.1
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
hanged.
>
>v2: rework based on further testing and suggestions from ivecera
>
>Fixes: a3b658cfb664 ("bonding: allow xfrm offload setup post-module-load")
>Reported-by: Ivan Vecera
>Suggested-by: Ivan Vecera
>Cc: Jay Vosburgh
>Cc: Veaceslav Falico
>Cc: Andy Gospodarek
t;, res);
Would it be better to add this call further down, after all
possible failures have been checked? I.e., if this new call to
bond_lower_state_changed() completes, and then very soon afterwards the
upper is unlinked, could that cause any issues?
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
Jarod Wilson wrote:
>Simply refer to what was the bonding "master" as the "bond" or bonding
>device, depending on context. However, do retain compat code for the
>bonding_masters sysfs interface to avoid breaking userspace.
>
>Cc: Jay Vosburgh
>Cc: Veace
ces entirely
>untouched, but update documentation to reference their deprecated
>nature, explain the name changes, add references to NetworkManager,
>include more netlink/iproute2 examples and make note of netlink
>being the preferred interface for userspace interaction with bonds.
>
ad, so outgoing traffic will
migrate from one bond port to another from time to time. It's unclear
to me how the described topology that's broken by the multicast traffic
being TX balanced is not also broken by the alb TX side rebalances.
-J
>The way the comparison is written now it does a single 64bit comparison
>per address, so it's the same number of instructions to compare the top
>two bytes or two full addresses.
---
-Jay Vosburgh, jay.vosbu...@canonical.com
*
>+ * Return true if the address is a multicast for IPv6.
>+ */
>+static inline bool is_ipv6_multicast_ether_addr(const u8 *addr)
>+{
>+ return (addr[0] & addr[1]) == 0x33;
>+}
I don't think this does what is intended. It will return true
for a MAC that starts with any two values whose bitwise AND is 0x33,
e.g., 0x73 0x3b. For IPv6 multicast, the first two octets of the MAC
must be exactly 0x33 0x33.
-J
>+
>+/**
> * is_valid_ether_addr - Determine if the given Ethernet address is valid
> * @addr: Pointer to a six-byte array containing the Ethernet address
> *
>--
>1.8.3.1
---
-Jay Vosburgh, jay.vosbu...@canonical.com
LIU Yulong wrote:
>Hi Jay,
>
>
>
>Thank you for your response and review. Please see my inline comments.
I'm still reviewing your commentary, but to answer your final
question regarding updating the patch, you'll need to repost the entire
patch (with the new changes). This repost shoul
one step than a piecemeal addition and removal from the
existing UAPI. That makes for a much clearer flag day event for end
users. By this I mean leave proc / sysfs as-is today, and then after a
suitable deprecation period, remove it wholesale (rather than a compile
time option).
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
Jarod Wilson wrote:
>On Tue, Sep 22, 2020 at 8:01 PM Stephen Hemminger
> wrote:
>>
>> On Tue, 22 Sep 2020 16:47:07 -0700
>> Jay Vosburgh wrote:
>>
>> > Stephen Hemminger wrote:
>> >
>> > >On Tue, 22 Sep 2020 09:37:30 -0400
>> &
enable, and
enabling will break the UAPI?
>Then you might convince maintainers to update documentation as well.
>Last I checked there were still references to ifenslave.
Distros still include ifenslave, but it's now a shell script
that uses sysfs. I see it used in scripts from time to time.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
. As far
>as I can tell, there is no breakage of existing interfaces with this
>set, unless the user consciously opts to do so via Kconfig.
>
>Jarod Wilson (5):
> bonding: rename struct slave member link to link_state
> bonding: rename slave to link where possible
> bondin
owest" here?
>Also, the type of the speed field is u32, not unsigned long, so adjust
>that accordingly, as required to make min() function here without
>complaining about mismatching types.
>
>Fixes: bb5b052f751b ("bond: add support to read speed and duplex via ethtool
Nivedita Singhvi wrote:
>Removed obsolete resources from bonding.rst doc:
> - bonding-de...@lists.sourceforge.net hasn't been used since 2008
> - admin interface is 404
> - Donald Becker's domain/content no longer online
>
>Signed-off-by: Nivedita Sing
have not tested or have familiarity with users using IP tunnel
interfaces with bonding as you're doing, so it's possible that some
aspect of that is interfering with the function of the ARP monitor.
-J
>uname -a:
>Linux fo-gw 4.19.0-9-amd64 #1 SMP Debian 4.19.118-2+deb10u1 (2020-06-07)
>x86_64 GNU/Linux
>(same on peer system)
>
>Am I misunderstanding how the driver works? Have I made any mistakes in
>the configuration?
>
>Best regards,
>Riccardo P. Bestetti
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
ve" within the bonding driver source?
This in addition to whatever API changes end up being done. If so, then
I would also like to know the answer to Andrew's question regarding
patch conflicts in order to gauge the future maintenance cost.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
is system),
>as well as successful failover and recovery mid-netperf.
>
>v2: rebase on latest net-next and wrap with #ifdef CONFIG_XFRM_OFFLOAD
>v3: add new CONFIG_BOND_XFRM_OFFLOAD option and fix shutdown path
>
>CC: Jay Vosburgh
>CC: Veaceslav Falico
>CC: Andy Gospodar
a similar problem.
>
>Fixes: 07699f9a7c8d ("bonding: add sysfs /slave dir for bond slave devices.")
>Signed-off-by: Qiushi Wu
Acked-by: Jay Vosburgh
>---
> drivers/net/bonding/bond_sysfs_slave.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff
)
>Reported-by: syzbot+e73ceacfd8560cc8a...@syzkaller.appspotmail.com
>Reported-by: syzbot+c2fb6f9ddcea95ba4...@syzkaller.appspotmail.com
>Cc: Nikolay Aleksandrov
>Cc: Josh Poimboeuf
>Cc: Jay Vosburgh
>Cc: Jann Horn
>Signed-off-by: Cong Wang
Reviewed-by: Jay Vosburgh
>---
> net/c
is sufficient to fix this
>issue.
>
>Fixes: fd867d51f889 ("net/core: generic support for disabling netdev features
>down stack")
>Reported-by: syzbot+e73ceacfd8560cc8a...@syzkaller.appspotmail.com
>Reported-by: syzbot+c2fb6f9ddcea95ba4...@syzkaller.appspotmail.com
>Cc:
lar to an issue from last fall; can you confirm
that you're running with a kernel that includes:
1899bb325149 bonding: fix state transition issue in link monitoring
-J
>CC: Jay Vosburgh
>CC: Veaceslav Falico
>CC: Andy Gospodarek
>Signed-off-by: Thomas Fa
Aleksei Zakharov wrote:
>чт, 26 сент. 2019 г. в 07:38, Jay Vosburgh :
>>
>> Aleksei Zakharov wrote:
>>
>> >ср, 25 сент. 2019 г. в 03:31, Jay Vosburgh :
>> >>
>> >> Алексей Захаров wrote:
>> >> [...]
>> >> >Right
ow who, or what it does.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
Aleksei Zakharov wrote:
>ср, 25 сент. 2019 г. в 03:31, Jay Vosburgh :
>>
>> Алексей Захаров wrote:
>> [...]
>> >Right after reboot one of the slaves hangs with actor port state 71
>> >and partner port state 1.
>> >It doesn't send lacpdu and s
of BOND_LINK_ */
- s8 new_link;
u8 backup:1, /* indicates backup slave. Value corresponds with
BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
inactive:1, /* indicates inactive slave */
@@ -549,7 +548,7 @@ static inline void bond_propose_link_state(struct slave
*slave, int state)
static inline void bond_commit_link_state(struct slave *slave, bool notify)
{
- if (slave->link == slave->link_new_state)
+ if (slave->link_new_state == BOND_LINK_NOCHANGE)
return;
slave->link = slave->link_new_state;
---
-Jay Vosburgh, jay.vosbu...@canonical.com
Алексей Захаров wrote:
>>
>> Jay Vosburgh wrote:
>> [...]
>> > In any event, I think I see what the failure is, I'm working up
>> >a patch to test and will post it when I have it ready.
>>
>> Aleksei,
>>
>>
Jay Vosburgh wrote:
[...]
> In any event, I think I see what the failure is, I'm working up
>a patch to test and will post it when I have it ready.
Aleksei,
Would you be able to test the following patch and see if it
resolves the issue in your testing? This
Алексей Захаров wrote:
>чт, 19 сент. 2019 г. в 11:00, Jay Vosburgh :
>>
>> Алексей Захаров wrote:
>>
>> >> >Once a while, one of 802.3ad slaves fails to initialize and hangs in
>> >> >BOND_LINK_FAIL state. Commit 334031219a84 ("bonding/
return 1;
+ }
switch (ecmd.base.duplex) {
case DUPLEX_FULL:
case DUPLEX_HALF:
break;
default:
+ pr_err("DBG duplex %u slave %s\n", ecmd.base.duplex,
+ slave_dev->name);
return 1;
}
@@ -3135,6 +3142,9 @@ static int bond_slave_netdev_event(unsigned long event,
*/
if (bond_update_speed_duplex(slave) &&
BOND_MODE(bond) == BOND_MODE_8023AD) {
+ pr_err("DBG slave %s event %d carrier %d\n",
+ slave->dev->name, event,
+ netif_carrier_ok(slave->dev));
if (slave->last_link_up)
slave->link = BOND_LINK_FAIL;
else
Thanks,
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
ce are you testing with that is exhibiting this behavior?
If I'm not mistaken, there have been several iterations of
hackery on this block of code to work around this same problem, and each
time there's some corner case that still doesn't work.
As Davem asked last time around, is the real problem that device
drivers report carrier up but supply invalid speed and duplex state?
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
times I see configurations or queries about an active-backup
bond with two 802.3ad bonding slaves. That particular configuration
doesn't have any advantage (802.3ad will internally manage that
situation), but I don't see that we can now forbid nesting bonds without
potentially breaking existing user space configurations.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
gt;[ 58.658476] ? netdev_change_features+0xa0/0xa0
>[ 58.663592] ? rtnl_create_link+0x2ed/0xad0
>[ 58.664049] bond_newlink+0x2a/0x60 [bonding]
>[ 58.664529] __rtnl_newlink+0xb9f/0x11b0
>[ 58.665014] ? rtnl_link_unregister+0x230/0x230
>[ ... ]
>
>Fixes: 0b680e75372
RENAME_OK = 1<<30,
>+ IFF_BONDING_SLAVE = 1<<31,
> };
>
> #define IFF_802_1Q_VLAN IFF_802_1Q_VLAN
>@@ -1539,6 +1541,7 @@ enum netdev_priv_flags {
> #define IFF_FAILOVER_SLAVEIFF_FAILOVER_SLAVE
> #define IFF_L3MDEV_RX_HANDLER IFF_L3MDEV_RX_HANDLER
> #define IFF_LIVE_RENAME_OKIFF_LIVE_RENAME_OK
>+#define IFF_BONDING_SLAVE IFF_BONDING_SLAVE
>
> /**
> *struct net_device - The DEVICE structure.
>@@ -4569,12 +4572,12 @@ static inline bool netif_is_macvlan_port(const struct
>net_device *dev)
>
> static inline bool netif_is_bond_master(const struct net_device *dev)
> {
>- return dev->flags & IFF_MASTER && dev->priv_flags & IFF_BONDING;
>+ return dev->priv_flags & IFF_BONDING;
> }
>
> static inline bool netif_is_bond_slave(const struct net_device *dev)
> {
>- return dev->flags & IFF_SLAVE && dev->priv_flags & IFF_BONDING;
>+ return dev->priv_flags & IFF_BONDING_SLAVE;
> }
>
> static inline bool netif_supports_nofcs(struct net_device *dev)
>--
>2.17.1
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
ze
in depth.
What I saw was that ping (IPv4) flood worked fine, bonded or
not, over a span of several hours. However, ping6 showed small numbers
of drops on a ping6 flood when bonded, on the order of 200 drops out of
48,000,000 requests sent. Zero losses when no bond in the stack. Both
tests to the same peer connected to the same switch. All of the above
with the bond using the Ethernet slave. I haven't tracked down where
those losses are occurring, so I don't know if it's on the transmit or
receive sides (or both).
I did this testing on a laptop with iwlwifi (Centrino 6205) and
an e1000e (82579LM) network device, using the a kernel built from
net-next as of earlier this week (it claimed to be 5.2.0-rc5+).
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
21865] [] ? rescuer_thread+0x380/0x380
>[12105.827757] [] ? kthread_associate_blkcg+0xc0/0xc0
>[12105.834266] [] ret_from_fork+0x51/0x60
>
>Fixes: e2a7420df2e0 ("bonding/main: convert to using slave printk macros")
>Signed-off-by: Eric Dumazet
>Reported-by: John S
Bill Carlson wrote:
>On 5/23/19 11:46 AM, Jay Vosburgh wrote:
>> As far as I'm aware, nesting bonds has no practical benefit; do
>> you have a use case for doing so?
>>
>>
>Use case is very specific, but needed in my situation until some switches
>are stabi
ed
the mailing list entirely at some point.
>Chasing whether a bond of bonds has an issue my testing hasn't revealed.
As far as I'm aware, nesting bonds has no practical benefit; do
you have a use case for doing so?
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
David Miller wrote:
>From: Jay Vosburgh
>Date: Mon, 13 May 2019 09:43:30 -0700
>
>> That would be my preference, as the 29c4948293bf commit looks to
>> be the change actually being fixed.
>
>Sorry I pushed the original commit message out :-(
>
>But isn&#
Jarod Wilson wrote:
>On 5/10/19 6:53 PM, Jay Vosburgh wrote:
>> Jarod Wilson wrote:
>>
>>> There's currently a problem with toggling arp_validate on and off with an
>>> active-backup bond. At the moment, you can start up a bond, like so:
>>&g
"bonding: always set recv_probe to bond_arp_rcv in arp
>monitor")
Is the above Fixes: tag correct? 3fe68df97c7f is not the source
of the erroneous logic being removed, which was introduced by
commit 29c4948293bfc426e52a921f4259eb3676961e81
Author: sfel...@cumulusnetworks.com
continue;
>@@ -3238,7 +3238,7 @@ static inline u32 bond_eth_hash(struct sk_buff *skb)
>
> ep = skb_header_pointer(skb, 0, sizeof(hdr_tmp), &hdr_tmp);
> if (ep)
>- return ep->h_dest[5] ^ ep->h_source[5] ^ ep->h_proto;
>+ retu
&newval);
thus, newval.value is initially be32, but stored in a u64.
__bond_opt_set will call bond_opt_parse, which in turn will call
bond_option_arp_ip_targets_set (via .set), and the change above would
swap the newval.value back to host order (on little endian architectures
for which cpu_to_be32 is not a no-op).
Am I misunderstanding? Did you test this change on an x86 or
other little endian system?
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
implementations like Open VSwitch and libreswitch, and
>vendor implementations like Arista EOS, seem to disable collecting +
>distributing to when doing similar port disabling/detaching/removing change.
>With this patch kernel implementation would behave the same way and ensure
>partner do
tate machine (figure 5-15) to exit C_D state and
go to ATTACHED state (disabling Coll/Dist).
But, either way, as this is a hint to get the link partner to
stop using the port, this looks reasonable to me.
Acked-by: Jay Vosburgh
-J
>Signed-off-by: Toni Peltonen
>---
d_actor_system),
>- &bond->params.ad_actor_system))
>+ ETH_ALEN, &bond->params.ad_actor_system))
> goto nla_put_failure;
> }
> if (!bond_3ad_get_active_agg_info(bond, &info)) {
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
Chas Williams <3ch...@gmail.com> wrote:
>On 10/25/2018 05:59 PM, Jay Vosburgh wrote:
>> Chas Williams <3ch...@gmail.com> wrote:
>>
>>> netif_is_lag_port should be used to identify link aggregation ports.
>>> For this to work, we need to reorganize
(dev->flags & IFF_SLAVE)
>+ if (netif_is_lag_port(dev) || dev->flags & IFF_SLAVE)
Note that netvsc_vf_join() also uses IFF_SLAVE in order skip
IPv6 addrconf for netvsc devices; I don't believe its usage will pass
netif_is_lag_port(). It looks like the above will work, but your commit
message mentions eql as the reason for retaining the IFF_SLAVE test, and
eql isn't the only user of IFF_SLAVE in this manner.
-J
> break;
>
> if (idev && idev->cnf.disable_ipv6)
>--
>2.14.4
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
or bonding, not team; otherwise LGTM.
Acked-by: Jay Vosburgh
-J
>Signed-off-by: Eric Dumazet
>Cc: Jay Vosburgh
>Cc: Veaceslav Falico
>Cc: Andy Gospodarek
>---
> drivers/net/bonding/bond_main.c | 11 ++-
> 1 file changed, 2 insertions(+), 9 deletions(-)
{
>+ struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
>+
>+ if (nskb) {
>+ nskb->dev = bond->dev;
>+ netif_rx(nskb);
>+ }
> return RX_HANDLER_PASS;
>+ }
> if (bond_should_deliver_exact_match(skb, slave, bond))
> return RX_HANDLER_EXACT;
>
>--
>2.18.0.203.gfac676dfb9-goog
---
-Jay Vosburgh, jay.vosbu...@canonical.com
Mahesh Bandewar (महेश बंडेवार) wrote:
>On Thu, Jul 12, 2018 at 11:03 AM, Jay Vosburgh
> wrote:
>> Michal Soltys wrote:
>>
>>>On 07/12/2018 04:51 PM, Jay Vosburgh wrote:
>>>> Mahesh Bandewar (महेश बंडेवार) wrote:
>>>>
>>>>> On We
Michal Soltys wrote:
>On 07/12/2018 04:51 PM, Jay Vosburgh wrote:
>> Mahesh Bandewar (महेश बंडेवार) wrote:
>>
>>> On Wed, Jul 11, 2018 at 3:23 PM, Michal Soltys wrote:
>>>>
>>>> Hi,
>>>>
>>>> As weird as that sounds, t
Apr 16 12:02:18 2017 -0700
bonding: deliver link-local packets with skb->dev set to link that packets
arrived on
Michal, are you able to revert this patch and test?
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
>index 8a945c9341d6..dba6cef05134 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -164,9 +164,10 @@ static const struct bond_opt_value
>bond_primary_reselect_tbl[] = {
> };
>
> static const struct bond_opt_value bond_use_carrier_tbl[] = {
>- { "off", 0, 0},
>- { "on", 1, BOND_VALFLAG_DEFAULT},
>- { NULL, -1, 0}
>+ { "off", 0, 0},
>+ { "on", 1, BOND_VALFLAG_DEFAULT},
>+ { "both", 2, 0},
>+ { NULL, -1, 0}
> };
>
> static const struct bond_opt_value bond_all_slaves_active_tbl[] = {
>--
>2.17.0
---
-Jay Vosburgh, jay.vosbu...@canonical.com
ivers/net/bonding/bond_options.c
>index 8a945c9341d6..dba6cef05134 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -164,9 +164,10 @@ static const struct bond_opt_value
>bond_primary_reselect_tbl[] = {
> };
>
> static const struct
Debabrata Banerjee wrote:
>The rx load balancing provided by balance-alb is not mutually
>exclusive with using hashing for tx selection, and should provide a decent
>speed increase because this eliminates spinlocks and cache contention.
>
>Signed-off-by: Debabrata Banerjee
>---
> drivers/net/bon
Banerjee, Debabrata wrote:
>> From: Jay Vosburgh [mailto:jay.vosbu...@canonical.com]
>> Debabrata Banerjee wrote:
>
>> >- if
>> (!ether_addr_equal_64bits(rx_hash_table[index].mac_dst,
>> >-
Debabrata Banerjee wrote:
>Replace homegrown mac addr checks with faster defs from etherdevice.h
>
>Signed-off-by: Debabrata Banerjee
>---
> drivers/net/bonding/bond_alb.c | 28 +---
> 1 file changed, 9 insertions(+), 19 deletions(-)
>
>diff --git a/drivers/net/bonding/bon
uot; VIRTIO_NET_F_STATUS
cases, and then the existing call to netif_carrier_on for the "without"
case will cause an operstate transition.
Cc: "Michael S. Tsirkin"
Cc: Jason Wang
Cc: Ben Hutchings
Signed-off-by: Jay Vosburgh
---
I considered resolving this by c
Michael S. Tsirkin wrote:
>On Thu, Mar 22, 2018 at 12:02:10PM +0000, Jay Vosburgh wrote:
>> Michael S. Tsirkin wrote:
>>
>> >On Thu, Mar 22, 2018 at 09:05:52AM +, Jay Vosburgh wrote:
>> >> The operstate update logic will leave an interface in the
&g
Michael S. Tsirkin wrote:
>On Thu, Mar 22, 2018 at 09:05:52AM +0000, Jay Vosburgh wrote:
>> The operstate update logic will leave an interface in the
>> default UNKNOWN operstate if the interface carrier state never changes
>> from the default carrier up state s
uot; VIRTIO_NET_F_STATUS
cases, and then the existing call to netif_carrier_on for the "without"
case will cause an operstate transition.
Cc: "Michael S. Tsirkin"
Cc: Jason Wang
Cc: Ben Hutchings
Fixes: 167c25e4c550 ("virtio-net: init link state correctly")
Signe
actually link up.
The remedy for this is to initialize new_link_state on each entry
to bond_miimon_inspect, as is already done with new_link.
Reported-by: Alex Sidorenko
Reviewed-by: Jarod Wilson
Signed-off-by: Jay Vosburgh
Fixes: fb9eb899a6dc ("bonding: handle link transition from
Jarod Wilson wrote:
>On 2017-11-02 9:11 PM, Jay Vosburgh wrote:
[...]
>> diff --git a/drivers/net/bonding/bond_main.c
>> b/drivers/net/bonding/bond_main.c
>> index 18b58e1376f1..6f89f9981a6c 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/
lave: new_active:
>ens3f1
>Oct 31 09:09:25 SYDC1LNX kernel: bond0: Non-BOND_LINK_BACK case
>Oct 31 09:09:25 SYDC1LNX kernel: bond0: making interface ens3f1 the new active
>one
>Oct 31 09:09:25 SYDC1LNX kernel: bond0: Setting MAC on new_active
>Oct 31 09:09:25 SYDC1LNX kernel: bo
Alex Sidorenko wrote:
>On 11/02/2017 12:51 AM, Jay Vosburgh wrote:
>> Jarod Wilson wrote:
>>
>>> On 2017-11-01 8:35 PM, Jay Vosburgh wrote:
>>>> Jay Vosburgh wrote:
>>>>
>>>>> Alex Sidorenko wrote:
>>>>>
Jarod Wilson wrote:
>On 2017-11-01 8:35 PM, Jay Vosburgh wrote:
>> Jay Vosburgh wrote:
>>
>>> Alex Sidorenko wrote:
>>>
>>>> The problem has been found while trying to deploy RHEL7 on HPE Synergy
>>>> platform, it is seen both in custom
Jay Vosburgh wrote:
>Alex Sidorenko wrote:
>
>>The problem has been found while trying to deploy RHEL7 on HPE Synergy
>>platform, it is seen both in customer's environment and in HPE test lab.
>>
>>There are several bonds configured in TLB mode and miimon=1
e where this is, but presuming that this is in the BOND_LINK_FAIL
case of the switch, it looks like both BOND_LINK_FAIL and BOND_LINK_BACK
will have the issue that if the link recovers or fails, respectively,
within the delay window (for down/updelay > 0) it won't set a
slave->new_link.
Looks like this got lost somewhere along the line, as originally
the transition back to UP (or DOWN) happened immediately, and that has
been lost somewhere.
I'll have to dig out when that broke, but I'll see about a test
patch this afternoon.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
se that absolutely
should just work. I don't think bonding cares (or should care) about
(a) - (c) for this use.
Your point (b) suggests that there are use cases other than the
above; I'm unfamiliar with any use other than wifi + ethernet, can you
elaborate?
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
In general, yes, but in this case, the condition should be
impossible to hit, so BUG_ON seems appropriate.
If bond_slave_get_rtnl/rcu() returns NULL for an actual bonding
slave, other code paths (bond_fill_slave_info, bond_handle_frame) will
likely crash before getting to this one.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
nto bond_option_active_slave_set for a slave prior to
bond_enslave registering the rx_handler for that slave, as these
operations are mutexed by RTNL.
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
ew_active);
This is a reasonable idea in principle, but will require
additional changes to prevent dereferencing new_active if it is NULL
(which would happen just below this point in the code).
-J
> if (new_active == old_active) {
> /* do nothing */
>--
>2.7.4
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
gt;> bond->params.arp_interval = 0;
>> /* set miimon to default value */
>> bond->params.miimon = BOND_DEFAULT_MIIMON;
>> -netdev_info(bond->dev, "Setting MII monitoring interval to
>> %d\n",
>> +netdev_dbg(bond->dev, "Setting MII monitoring interval to %d\n",
>> bond->params.miimon);
>
>etc...
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
nding/bond_main.c
>@@ -2614,8 +2614,7 @@ static void bond_loadbalance_arp_mon(struct
>work_struct *work)
>rcu_read_unlock();
>
>if (do_failover || slave_state_changed) {
>- if (!rtnl_trylock())
>- goto re_arm;
>+ rtnl_lock();
>
>if (slave_state_changed) {
>bond_slave_state_change(bond);
>
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
1407 ("[PATCH] bonding: Handle large hard_header_len")
>Fixes: fc791b633515 ("IB/ipoib: move back IB LL address into the hard header")
>Signed-off-by: Marcelo Ricardo Leitner
>Signed-off-by: Paolo Abeni
Signed-off-by: Jay Vosburgh
> drivers/net/bonding/bond_main.c
bonding-master. This
>patch changes this behavior for link-local packets by not changing
>the skb->dev to the bonding-master and maintaining it as it is,
>i.e. the link on which the packet arrived.
Minor nit: "looses" should be "loses". Other than that:
Mahesh Bandewar (महेश बंडेवार) wrote:
>On Fri, Apr 14, 2017 at 12:30 PM, Jay Vosburgh
> wrote:
>>
>>
>> Chonggang Li wrote:
>>
>> >Previously link-local packets excluding LACP (which are handled by
>> >the recv_probe) received on bond slave interf
if (bond_should_deliver_exact_match(skb, slave, bond))
> return RX_HANDLER_EXACT;
>
>--
>2.12.2.762.g0e3151a226-goog
>
---
-Jay Vosburgh, jay.vosbu...@canonical.com
s
format string. Perhaps that format would be a better choice than %pM
for this case?
-J
---
-Jay Vosburgh, jay.vosbu...@canonical.com
te
>from the LACP state machine. This state is defined but never set so
>removing from the state machine logic makes code little cleaner.
>
>(d) Reduce scope of some global variables to local.
For all patches in the series:
Signed-off-by: Jay Vosburgh
>N
1 - 100 of 405 matches
Mail list logo
