* Re: [LKP] [net] 4ed2d765dfa: ltp.recv01.4.TFAIL
From: Willem de Bruijn @ 2014-10-01 15:57 UTC (permalink / raw)
To: Fengguang Wu; +Cc: Dave Hansen, Network Development, LKML, lkp
In-Reply-To: <20141001152910.GA4440@wfg-t540p.sh.intel.com>
On Wed, Oct 1, 2014 at 11:29 AM, Fengguang Wu <fengguang.wu@intel.com> wrote:
> Hi Willem,
>
> FYI, we noticed the below LTP failures on commit
Thanks for the report, Fengguang. The failures are
recv01 4 TFAIL : recv01.c:142: invalid flags set ; returned -1
(expected -1), errno 11 (expected 22)
recvfrom01 4 TFAIL : recvfrom01.c:164: invalid socket length ;
returned -1 (expected -1), errno 11 (expected 22)
recvfrom01 6 TFAIL : recvfrom01.c:164: invalid flags set ;
returned -1 (expected -1), errno 11 (expected 22)
recvmsg01 9 TFAIL : recvmsg01.c:228: invalid flags set ;
returned -1 (expected -1), errno 11 (expected 22)
In other words, these functions return EAGAIN now, when they used to
return EINVAL. This will happen from this patch onwards if flags
includes MSG_ERRQUEUE. The patch introduced error queue support for
TCP sockets. Like other recvmsg implementations, the error queue code
will act on some flags and ignore others. Previously, the code
happened to check MSG_OOB first and return EINVAL. The error queue
code ignores this flag and returns EAGAIN if no data is waiting.
This was recently discussed in
LTP recv/recvmsg tests failing on 3.17
http://comments.gmane.org/gmane.linux.network/331750
where the consensus was that applications should not expect
particular error codes when passing unsupported combinations
of flags.
^ permalink raw reply
* Re: [PATCH net-next v6 2/2] bonding: Simplify the xmit function for modes that use xmit_hash
From: Jay Vosburgh @ 2014-10-01 16:19 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Veaceslav Falico, Andy Gospodarek, David Miller, netdev,
Eric Dumazet, Maciej Zenczykowski
In-Reply-To: <1412152711-12646-1-git-send-email-maheshb@google.com>
Mahesh Bandewar <maheshb@google.com> wrote:
>Earlier change to use usable slave array for TLB mode had an additional
>performance advantage. So extending the same logic to all other modes
>that use xmit-hash for slave selection (viz 802.3AD, and XOR modes).
>Also consolidating this with the earlier TLB change.
>
>The main idea is to build the usable slaves array in the control path
>and use that array for slave selection during xmit operation.
>
>Measured performance in a setup with a bond of 4x1G NICs with 200
>instances of netperf for the modes involved (3ad, xor, tlb)
>cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5
>
>Mode TPS-Before TPS-After
>
>802.3ad : 468,694 493,101
>TLB (lb=0): 392,583 392,965
>XOR : 475,696 484,517
>
>Signed-off-by: Mahesh Bandewar <maheshb@google.com>
>---
>v1:
> (a) If bond_update_slave_arr() fails to allocate memory, it will overwrite
> the slave that need to be removed.
> (b) Freeing of array will assign NULL (to handle bond->down to bond->up
> transition gracefully.
> (c) Change from pr_debug() to pr_err() if bond_update_slave_arr() returns
> failure.
> (d) XOR: bond_update_slave_arr() will consider mii-mon, arp-mon cases and
> will populate the array even if these parameters are not used.
> (e) 3AD: Should handle the ad_agg_selection_logic correctly.
>v2:
> (a) Removed rcu_read_{un}lock() calls from array manipulation code.
> (b) Slave link-events now refresh array for all these modes.
> (c) Moved free-array call from bond_close() to bond_uninit().
>v3:
> (a) Fixed null pointer dereference.
> (b) Removed bond->lock lockdep dependency.
>v4:
> (a) Made to changes to comply with Nikolay's locking changes
> (b) Added a work-queue to refresh slave-array when RTNL is not held
> (c) Array refresh happens ONLY with RTNL now.
> (d) alloc changed from GFP_ATOMIC to GFP_KERNEL
>v5:
> (a) Consolidated all delayed slave-array updates at one place in
> 3ad_state_machine_handler()
>v6:
> (a) Free slave array when there is no active aggregator
>
> drivers/net/bonding/bond_3ad.c | 140 +++++++++++------------------
> drivers/net/bonding/bond_alb.c | 51 ++---------
> drivers/net/bonding/bond_alb.h | 8 --
> drivers/net/bonding/bond_main.c | 192 +++++++++++++++++++++++++++++++++++++---
> drivers/net/bonding/bonding.h | 10 +++
> 5 files changed, 249 insertions(+), 152 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index 7e9e522fd476..2110215f3528 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -102,17 +102,20 @@ static const u8 lacpdu_mcast_addr[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
> /* ================= main 802.3ad protocol functions ================== */
> static int ad_lacpdu_send(struct port *port);
> static int ad_marker_send(struct port *port, struct bond_marker *marker);
>-static void ad_mux_machine(struct port *port);
>+static void ad_mux_machine(struct port *port, bool *update_slave_arr);
> static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port);
> static void ad_tx_machine(struct port *port);
> static void ad_periodic_machine(struct port *port);
>-static void ad_port_selection_logic(struct port *port);
>-static void ad_agg_selection_logic(struct aggregator *aggregator);
>+static void ad_port_selection_logic(struct port *port, bool *update_slave_arr);
>+static void ad_agg_selection_logic(struct aggregator *aggregator,
>+ bool *update_slave_arr);
> static void ad_clear_agg(struct aggregator *aggregator);
> static void ad_initialize_agg(struct aggregator *aggregator);
> static void ad_initialize_port(struct port *port, int lacp_fast);
>-static void ad_enable_collecting_distributing(struct port *port);
>-static void ad_disable_collecting_distributing(struct port *port);
>+static void ad_enable_collecting_distributing(struct port *port,
>+ bool *update_slave_arr);
>+static void ad_disable_collecting_distributing(struct port *port,
>+ bool *update_slave_arr);
> static void ad_marker_info_received(struct bond_marker *marker_info,
> struct port *port);
> static void ad_marker_response_received(struct bond_marker *marker,
>@@ -796,8 +799,9 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
> /**
> * ad_mux_machine - handle a port's mux state machine
> * @port: the port we're looking at
>+ * @update_slave_arr: Does slave array need update?
> */
>-static void ad_mux_machine(struct port *port)
>+static void ad_mux_machine(struct port *port, bool *update_slave_arr)
> {
> mux_states_t last_state;
>
>@@ -901,7 +905,8 @@ static void ad_mux_machine(struct port *port)
> switch (port->sm_mux_state) {
> case AD_MUX_DETACHED:
> port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
>- ad_disable_collecting_distributing(port);
>+ ad_disable_collecting_distributing(port,
>+ update_slave_arr);
> port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
> port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
> port->ntt = true;
>@@ -913,13 +918,15 @@ static void ad_mux_machine(struct port *port)
> port->actor_oper_port_state |= AD_STATE_SYNCHRONIZATION;
> port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
> port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
>- ad_disable_collecting_distributing(port);
>+ ad_disable_collecting_distributing(port,
>+ update_slave_arr);
> port->ntt = true;
> break;
> case AD_MUX_COLLECTING_DISTRIBUTING:
> port->actor_oper_port_state |= AD_STATE_COLLECTING;
> port->actor_oper_port_state |= AD_STATE_DISTRIBUTING;
>- ad_enable_collecting_distributing(port);
>+ ad_enable_collecting_distributing(port,
>+ update_slave_arr);
> port->ntt = true;
> break;
> default:
>@@ -1187,12 +1194,13 @@ static void ad_periodic_machine(struct port *port)
> /**
> * ad_port_selection_logic - select aggregation groups
> * @port: the port we're looking at
>+ * @update_slave_arr: Does slave array need update?
> *
> * Select aggregation groups, and assign each port for it's aggregetor. The
> * selection logic is called in the inititalization (after all the handshkes),
> * and after every lacpdu receive (if selected is off).
> */
>-static void ad_port_selection_logic(struct port *port)
>+static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
Since this function is void, why not have it return a value
instead of the bool *update_slave_arr? That would eliminate the need
for some call sites to pass a "dummy" to the function. This comment
applies to ad_agg_selection_logic and ad_enable_collecting_distributing
as well.
-J
> {
> struct aggregator *aggregator, *free_aggregator = NULL, *temp_aggregator;
> struct port *last_port = NULL, *curr_port;
>@@ -1347,7 +1355,7 @@ static void ad_port_selection_logic(struct port *port)
> __agg_ports_are_ready(port->aggregator));
>
> aggregator = __get_first_agg(port);
>- ad_agg_selection_logic(aggregator);
>+ ad_agg_selection_logic(aggregator, update_slave_arr);
> }
>
> /* Decide if "agg" is a better choice for the new active aggregator that
>@@ -1435,6 +1443,7 @@ static int agg_device_up(const struct aggregator *agg)
> /**
> * ad_agg_selection_logic - select an aggregation group for a team
> * @aggregator: the aggregator we're looking at
>+ * @update_slave_arr: Does slave array need update?
> *
> * It is assumed that only one aggregator may be selected for a team.
> *
>@@ -1457,7 +1466,8 @@ static int agg_device_up(const struct aggregator *agg)
> * __get_active_agg() won't work correctly. This function should be better
> * called with the bond itself, and retrieve the first agg from it.
> */
>-static void ad_agg_selection_logic(struct aggregator *agg)
>+static void ad_agg_selection_logic(struct aggregator *agg,
>+ bool *update_slave_arr)
> {
> struct aggregator *best, *active, *origin;
> struct bonding *bond = agg->slave->bond;
>@@ -1550,6 +1560,8 @@ static void ad_agg_selection_logic(struct aggregator *agg)
> __disable_port(port);
> }
> }
>+ /* Slave array needs update. */
>+ *update_slave_arr = true;
> }
>
> /* if the selected aggregator is of join individuals
>@@ -1678,24 +1690,30 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
> /**
> * ad_enable_collecting_distributing - enable a port's transmit/receive
> * @port: the port we're looking at
>+ * @update_slave_arr: Does slave array need update?
> *
> * Enable @port if it's in an active aggregator
> */
>-static void ad_enable_collecting_distributing(struct port *port)
>+static void ad_enable_collecting_distributing(struct port *port,
>+ bool *update_slave_arr)
> {
> if (port->aggregator->is_active) {
> pr_debug("Enabling port %d(LAG %d)\n",
> port->actor_port_number,
> port->aggregator->aggregator_identifier);
> __enable_port(port);
>+ /* Slave array needs update */
>+ *update_slave_arr = true;
> }
> }
>
> /**
> * ad_disable_collecting_distributing - disable a port's transmit/receive
> * @port: the port we're looking at
>+ * @update_slave_arr: Does slave array need update?
> */
>-static void ad_disable_collecting_distributing(struct port *port)
>+static void ad_disable_collecting_distributing(struct port *port,
>+ bool *update_slave_arr)
> {
> if (port->aggregator &&
> !MAC_ADDRESS_EQUAL(&(port->aggregator->partner_system),
>@@ -1704,6 +1722,8 @@ static void ad_disable_collecting_distributing(struct port *port)
> port->actor_port_number,
> port->aggregator->aggregator_identifier);
> __disable_port(port);
>+ /* Slave array needs an update */
>+ *update_slave_arr = true;
> }
> }
>
>@@ -1868,6 +1888,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
> struct bonding *bond = slave->bond;
> struct slave *slave_iter;
> struct list_head *iter;
>+ bool dummy_slave_update; /* Ignore this value as caller updates array */
>
> /* Sync against bond_3ad_state_machine_handler() */
> spin_lock_bh(&bond->mode_lock);
>@@ -1951,7 +1972,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
> ad_clear_agg(aggregator);
>
> if (select_new_active_agg)
>- ad_agg_selection_logic(__get_first_agg(port));
>+ ad_agg_selection_logic(__get_first_agg(port),
>+ &dummy_slave_update);
> } else {
> netdev_warn(bond->dev, "unbinding aggregator, and could not find a new aggregator for its ports\n");
> }
>@@ -1966,7 +1988,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
> /* select new active aggregator */
> temp_aggregator = __get_first_agg(port);
> if (temp_aggregator)
>- ad_agg_selection_logic(temp_aggregator);
>+ ad_agg_selection_logic(temp_aggregator,
>+ &dummy_slave_update);
> }
> }
> }
>@@ -1996,7 +2019,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
> if (select_new_active_agg) {
> netdev_info(bond->dev, "Removing an active aggregator\n");
> /* select new active aggregator */
>- ad_agg_selection_logic(__get_first_agg(port));
>+ ad_agg_selection_logic(__get_first_agg(port),
>+ &dummy_slave_update);
> }
> }
> break;
>@@ -2031,6 +2055,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
> struct slave *slave;
> struct port *port;
> bool should_notify_rtnl = BOND_SLAVE_NOTIFY_LATER;
>+ bool update_slave_arr = false;
>
> /* Lock to protect data accessed by all (e.g., port->sm_vars) and
> * against running with bond_3ad_unbind_slave. ad_rx_machine may run
>@@ -2058,7 +2083,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
> }
>
> aggregator = __get_first_agg(port);
>- ad_agg_selection_logic(aggregator);
>+ ad_agg_selection_logic(aggregator, &update_slave_arr);
> }
> bond_3ad_set_carrier(bond);
> }
>@@ -2074,8 +2099,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
>
> ad_rx_machine(NULL, port);
> ad_periodic_machine(port);
>- ad_port_selection_logic(port);
>- ad_mux_machine(port);
>+ ad_port_selection_logic(port, &update_slave_arr);
>+ ad_mux_machine(port, &update_slave_arr);
> ad_tx_machine(port);
>
> /* turn off the BEGIN bit, since we already handled it */
>@@ -2093,6 +2118,9 @@ re_arm:
> rcu_read_unlock();
> spin_unlock_bh(&bond->mode_lock);
>
>+ if (update_slave_arr)
>+ bond_slave_arr_work_rearm(bond, 0);
>+
> if (should_notify_rtnl && rtnl_trylock()) {
> bond_slave_state_notify(bond);
> rtnl_unlock();
>@@ -2283,6 +2311,11 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
> port->sm_vars |= AD_PORT_BEGIN;
>
> spin_unlock_bh(&slave->bond->mode_lock);
>+
>+ /* RTNL is held and mode_lock is released so it's safe
>+ * to update slave_array here.
>+ */
>+ bond_update_slave_arr(slave->bond, NULL);
> }
>
> /**
>@@ -2377,73 +2410,6 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
> return ret;
> }
>
>-int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
>-{
>- struct bonding *bond = netdev_priv(dev);
>- struct slave *slave, *first_ok_slave;
>- struct aggregator *agg;
>- struct ad_info ad_info;
>- struct list_head *iter;
>- int slaves_in_agg;
>- int slave_agg_no;
>- int agg_id;
>-
>- if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
>- netdev_dbg(dev, "__bond_3ad_get_active_agg_info failed\n");
>- goto err_free;
>- }
>-
>- slaves_in_agg = ad_info.ports;
>- agg_id = ad_info.aggregator_id;
>-
>- if (slaves_in_agg == 0) {
>- netdev_dbg(dev, "active aggregator is empty\n");
>- goto err_free;
>- }
>-
>- slave_agg_no = bond_xmit_hash(bond, skb) % slaves_in_agg;
>- first_ok_slave = NULL;
>-
>- bond_for_each_slave_rcu(bond, slave, iter) {
>- agg = SLAVE_AD_INFO(slave)->port.aggregator;
>- if (!agg || agg->aggregator_identifier != agg_id)
>- continue;
>-
>- if (slave_agg_no >= 0) {
>- if (!first_ok_slave && bond_slave_can_tx(slave))
>- first_ok_slave = slave;
>- slave_agg_no--;
>- continue;
>- }
>-
>- if (bond_slave_can_tx(slave)) {
>- bond_dev_queue_xmit(bond, skb, slave->dev);
>- goto out;
>- }
>- }
>-
>- if (slave_agg_no >= 0) {
>- netdev_err(dev, "Couldn't find a slave to tx on for aggregator ID %d\n",
>- agg_id);
>- goto err_free;
>- }
>-
>- /* we couldn't find any suitable slave after the agg_no, so use the
>- * first suitable found, if found.
>- */
>- if (first_ok_slave)
>- bond_dev_queue_xmit(bond, skb, first_ok_slave->dev);
>- else
>- goto err_free;
>-
>-out:
>- return NETDEV_TX_OK;
>-err_free:
>- /* no suitable interface, frame not sent */
>- dev_kfree_skb_any(skb);
>- goto out;
>-}
>-
> int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
> struct slave *slave)
> {
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index 615f3bebd019..d2eadab787c5 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -177,7 +177,6 @@ static int tlb_initialize(struct bonding *bond)
> static void tlb_deinitialize(struct bonding *bond)
> {
> struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
>- struct tlb_up_slave *arr;
>
> spin_lock_bh(&bond->mode_lock);
>
>@@ -185,10 +184,6 @@ static void tlb_deinitialize(struct bonding *bond)
> bond_info->tx_hashtbl = NULL;
>
> spin_unlock_bh(&bond->mode_lock);
>-
>- arr = rtnl_dereference(bond_info->slave_arr);
>- if (arr)
>- kfree_rcu(arr, rcu);
> }
>
> static long long compute_gap(struct slave *slave)
>@@ -1336,39 +1331,9 @@ out:
> return NETDEV_TX_OK;
> }
>
>-static int bond_tlb_update_slave_arr(struct bonding *bond,
>- struct slave *skipslave)
>-{
>- struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
>- struct slave *tx_slave;
>- struct list_head *iter;
>- struct tlb_up_slave *new_arr, *old_arr;
>-
>- new_arr = kzalloc(offsetof(struct tlb_up_slave, arr[bond->slave_cnt]),
>- GFP_ATOMIC);
>- if (!new_arr)
>- return -ENOMEM;
>-
>- bond_for_each_slave(bond, tx_slave, iter) {
>- if (!bond_slave_can_tx(tx_slave))
>- continue;
>- if (skipslave == tx_slave)
>- continue;
>- new_arr->arr[new_arr->count++] = tx_slave;
>- }
>-
>- old_arr = rtnl_dereference(bond_info->slave_arr);
>- rcu_assign_pointer(bond_info->slave_arr, new_arr);
>- if (old_arr)
>- kfree_rcu(old_arr, rcu);
>-
>- return 0;
>-}
>-
> int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
> {
> struct bonding *bond = netdev_priv(bond_dev);
>- struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
> struct ethhdr *eth_data;
> struct slave *tx_slave = NULL;
> u32 hash_index;
>@@ -1389,12 +1354,14 @@ int bond_tlb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
> hash_index & 0xFF,
> skb->len);
> } else {
>- struct tlb_up_slave *slaves;
>+ struct bond_up_slave *slaves;
>+ unsigned int count;
>
>- slaves = rcu_dereference(bond_info->slave_arr);
>- if (slaves && slaves->count)
>+ slaves = rcu_dereference(bond->slave_arr);
>+ count = slaves ? ACCESS_ONCE(slaves->count) : 0;
>+ if (likely(count))
> tx_slave = slaves->arr[hash_index %
>- slaves->count];
>+ count];
> }
> break;
> }
>@@ -1641,10 +1608,6 @@ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave)
> rlb_clear_slave(bond, slave);
> }
>
>- if (bond_is_nondyn_tlb(bond))
>- if (bond_tlb_update_slave_arr(bond, slave))
>- pr_err("Failed to build slave-array for TLB mode.\n");
>-
> }
>
> void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char link)
>@@ -1669,7 +1632,7 @@ void bond_alb_handle_link_change(struct bonding *bond, struct slave *slave, char
> }
>
> if (bond_is_nondyn_tlb(bond)) {
>- if (bond_tlb_update_slave_arr(bond, NULL))
>+ if (bond_update_slave_arr(bond, NULL))
> pr_err("Failed to build slave-array for TLB mode.\n");
> }
> }
>diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
>index 3c6a7ff974d7..1ad473b4ade5 100644
>--- a/drivers/net/bonding/bond_alb.h
>+++ b/drivers/net/bonding/bond_alb.h
>@@ -139,19 +139,11 @@ struct tlb_slave_info {
> */
> };
>
>-struct tlb_up_slave {
>- unsigned int count;
>- struct rcu_head rcu;
>- struct slave *arr[0];
>-};
>-
> struct alb_bond_info {
> struct tlb_client_info *tx_hashtbl; /* Dynamically allocated */
> u32 unbalanced_load;
> int tx_rebalance_counter;
> int lp_counter;
>- /* -------- non-dynamic tlb mode only ---------*/
>- struct tlb_up_slave __rcu *slave_arr; /* Up slaves */
> /* -------- rlb parameters -------- */
> int rlb_enabled;
> struct rlb_client_info *rx_hashtbl; /* Receive hash table */
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index c2adc2755ff6..6f79f495b01a 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -210,6 +210,7 @@ static int bond_init(struct net_device *bond_dev);
> static void bond_uninit(struct net_device *bond_dev);
> static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
> struct rtnl_link_stats64 *stats);
>+static void bond_slave_arr_handler(struct work_struct *work);
>
> /*---------------------------- General routines -----------------------------*/
>
>@@ -1551,6 +1552,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> unblock_netpoll_tx();
> }
>
>+ if (bond_mode_uses_xmit_hash(bond))
>+ bond_update_slave_arr(bond, NULL);
>+
> netdev_info(bond_dev, "Enslaving %s as %s interface with %s link\n",
> slave_dev->name,
> bond_is_active_slave(new_slave) ? "an active" : "a backup",
>@@ -1668,6 +1672,9 @@ static int __bond_release_one(struct net_device *bond_dev,
> if (BOND_MODE(bond) == BOND_MODE_8023AD)
> bond_3ad_unbind_slave(slave);
>
>+ if (bond_mode_uses_xmit_hash(bond))
>+ bond_update_slave_arr(bond, slave);
>+
> netdev_info(bond_dev, "Releasing %s interface %s\n",
> bond_is_active_slave(slave) ? "active" : "backup",
> slave_dev->name);
>@@ -1970,6 +1977,9 @@ static void bond_miimon_commit(struct bonding *bond)
> bond_alb_handle_link_change(bond, slave,
> BOND_LINK_UP);
>
>+ if (BOND_MODE(bond) == BOND_MODE_XOR)
>+ bond_update_slave_arr(bond, NULL);
>+
> if (!bond->curr_active_slave || slave == primary)
> goto do_failover;
>
>@@ -1997,6 +2007,9 @@ static void bond_miimon_commit(struct bonding *bond)
> bond_alb_handle_link_change(bond, slave,
> BOND_LINK_DOWN);
>
>+ if (BOND_MODE(bond) == BOND_MODE_XOR)
>+ bond_update_slave_arr(bond, NULL);
>+
> if (slave == rcu_access_pointer(bond->curr_active_slave))
> goto do_failover;
>
>@@ -2453,6 +2466,8 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
>
> if (slave_state_changed) {
> bond_slave_state_change(bond);
>+ if (BOND_MODE(bond) == BOND_MODE_XOR)
>+ bond_update_slave_arr(bond, NULL);
> } else if (do_failover) {
> block_netpoll_tx();
> bond_select_active_slave(bond);
>@@ -2829,8 +2844,20 @@ static int bond_slave_netdev_event(unsigned long event,
> if (old_duplex != slave->duplex)
> bond_3ad_adapter_duplex_changed(slave);
> }
>+ /* Refresh slave-array if applicable!
>+ * If the setup does not use miimon or arpmon (mode-specific!),
>+ * then these events will not cause the slave-array to be
>+ * refreshed. This will cause xmit to use a slave that is not
>+ * usable. Avoid such situation by refeshing the array at these
>+ * events. If these (miimon/arpmon) parameters are configured
>+ * then array gets refreshed twice and that should be fine!
>+ */
>+ if (bond_mode_uses_xmit_hash(bond))
>+ bond_update_slave_arr(bond, NULL);
> break;
> case NETDEV_DOWN:
>+ if (bond_mode_uses_xmit_hash(bond))
>+ bond_update_slave_arr(bond, NULL);
> break;
> case NETDEV_CHANGEMTU:
> /* TODO: Should slaves be allowed to
>@@ -3010,6 +3037,7 @@ static void bond_work_init_all(struct bonding *bond)
> else
> INIT_DELAYED_WORK(&bond->arp_work, bond_loadbalance_arp_mon);
> INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
>+ INIT_DELAYED_WORK(&bond->slave_arr_work, bond_slave_arr_handler);
> }
>
> static void bond_work_cancel_all(struct bonding *bond)
>@@ -3019,6 +3047,7 @@ static void bond_work_cancel_all(struct bonding *bond)
> cancel_delayed_work_sync(&bond->alb_work);
> cancel_delayed_work_sync(&bond->ad_work);
> cancel_delayed_work_sync(&bond->mcast_work);
>+ cancel_delayed_work_sync(&bond->slave_arr_work);
> }
>
> static int bond_open(struct net_device *bond_dev)
>@@ -3068,6 +3097,9 @@ static int bond_open(struct net_device *bond_dev)
> bond_3ad_initiate_agg_selection(bond, 1);
> }
>
>+ if (bond_mode_uses_xmit_hash(bond))
>+ bond_update_slave_arr(bond, NULL);
>+
> return 0;
> }
>
>@@ -3573,20 +3605,148 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
> return NETDEV_TX_OK;
> }
>
>-/* In bond_xmit_xor() , we determine the output device by using a pre-
>- * determined xmit_hash_policy(), If the selected device is not enabled,
>- * find the next active slave.
>+/* Use this to update slave_array when (a) it's not appropriate to update
>+ * slave_array right away (note that update_slave_array() may sleep)
>+ * and / or (b) RTNL is not held.
> */
>-static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
>+void bond_slave_arr_work_rearm(struct bonding *bond, unsigned long delay)
> {
>- struct bonding *bond = netdev_priv(bond_dev);
>- int slave_cnt = ACCESS_ONCE(bond->slave_cnt);
>+ queue_delayed_work(bond->wq, &bond->slave_arr_work, delay);
>+}
>
>- if (likely(slave_cnt))
>- bond_xmit_slave_id(bond, skb,
>- bond_xmit_hash(bond, skb) % slave_cnt);
>- else
>+/* Slave array work handler. Holds only RTNL */
>+static void bond_slave_arr_handler(struct work_struct *work)
>+{
>+ struct bonding *bond = container_of(work, struct bonding,
>+ slave_arr_work.work);
>+ int ret;
>+
>+ if (!rtnl_trylock())
>+ goto err;
>+
>+ ret = bond_update_slave_arr(bond, NULL);
>+ rtnl_unlock();
>+ if (ret) {
>+ pr_warn_ratelimited("Failed to update slave array from WT\n");
>+ goto err;
>+ }
>+ return;
>+
>+err:
>+ bond_slave_arr_work_rearm(bond, 1);
>+}
>+
>+/* Build the usable slaves array in control path for modes that use xmit-hash
>+ * to determine the slave interface -
>+ * (a) BOND_MODE_8023AD
>+ * (b) BOND_MODE_XOR
>+ * (c) BOND_MODE_TLB && tlb_dynamic_lb == 0
>+ *
>+ * The caller is expected to hold RTNL only and NO other lock!
>+ */
>+int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
>+{
>+ struct slave *slave;
>+ struct list_head *iter;
>+ struct bond_up_slave *new_arr, *old_arr;
>+ int slaves_in_agg;
>+ int agg_id = 0;
>+ int ret = 0;
>+
>+#ifdef CONFIG_LOCKDEP
>+ WARN_ON(lockdep_is_held(&bond->mode_lock));
>+#endif
>+
>+ new_arr = kzalloc(offsetof(struct bond_up_slave, arr[bond->slave_cnt]),
>+ GFP_KERNEL);
>+ if (!new_arr) {
>+ ret = -ENOMEM;
>+ pr_err("Failed to build slave-array.\n");
>+ goto out;
>+ }
>+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>+ struct ad_info ad_info;
>+
>+ if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
>+ pr_debug("bond_3ad_get_active_agg_info failed\n");
>+ kfree_rcu(new_arr, rcu);
>+ /* No active aggragator means its not safe to use
>+ * the previous array.
>+ */
>+ old_arr = rtnl_dereference(bond->slave_arr);
>+ if (old_arr) {
>+ RCU_INIT_POINTER(bond->slave_arr, NULL);
>+ kfree_rcu(old_arr, rcu);
>+ }
>+ goto out;
>+ }
>+ slaves_in_agg = ad_info.ports;
>+ agg_id = ad_info.aggregator_id;
>+ }
>+ bond_for_each_slave(bond, slave, iter) {
>+ if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>+ struct aggregator *agg;
>+
>+ agg = SLAVE_AD_INFO(slave)->port.aggregator;
>+ if (!agg || agg->aggregator_identifier != agg_id)
>+ continue;
>+ }
>+ if (!bond_slave_can_tx(slave))
>+ continue;
>+ if (skipslave == slave)
>+ continue;
>+ new_arr->arr[new_arr->count++] = slave;
>+ }
>+
>+ old_arr = rtnl_dereference(bond->slave_arr);
>+ rcu_assign_pointer(bond->slave_arr, new_arr);
>+ if (old_arr)
>+ kfree_rcu(old_arr, rcu);
>+out:
>+ if (ret != 0 && skipslave) {
>+ int idx;
>+
>+ /* Rare situation where caller has asked to skip a specific
>+ * slave but allocation failed (most likely!). BTW this is
>+ * only possible when the call is initiated from
>+ * __bond_release_one(). In this situation; overwrite the
>+ * skipslave entry in the array with the last entry from the
>+ * array to avoid a situation where the xmit path may choose
>+ * this to-be-skipped slave to send a packet out.
>+ */
>+ old_arr = rtnl_dereference(bond->slave_arr);
>+ for (idx = 0; idx < old_arr->count; idx++) {
>+ if (skipslave == old_arr->arr[idx]) {
>+ old_arr->arr[idx] =
>+ old_arr->arr[old_arr->count-1];
>+ old_arr->count--;
>+ break;
>+ }
>+ }
>+ }
>+ return ret;
>+}
>+
>+/* Use this Xmit function for 3AD as well as XOR modes. The current
>+ * usable slave array is formed in the control path. The xmit function
>+ * just calculates hash and sends the packet out.
>+ */
>+int bond_3ad_xor_xmit(struct sk_buff *skb, struct net_device *dev)
>+{
>+ struct bonding *bond = netdev_priv(dev);
>+ struct slave *slave;
>+ struct bond_up_slave *slaves;
>+ unsigned int count;
>+
>+ slaves = rcu_dereference(bond->slave_arr);
>+ count = slaves ? ACCESS_ONCE(slaves->count) : 0;
>+ if (likely(count)) {
>+ slave = slaves->arr[bond_xmit_hash(bond, skb) % count];
>+ bond_dev_queue_xmit(bond, skb, slave->dev);
>+ } else {
> dev_kfree_skb_any(skb);
>+ atomic_long_inc(&dev->tx_dropped);
>+ }
>
> return NETDEV_TX_OK;
> }
>@@ -3682,12 +3842,11 @@ static netdev_tx_t __bond_start_xmit(struct sk_buff *skb, struct net_device *dev
> return bond_xmit_roundrobin(skb, dev);
> case BOND_MODE_ACTIVEBACKUP:
> return bond_xmit_activebackup(skb, dev);
>+ case BOND_MODE_8023AD:
> case BOND_MODE_XOR:
>- return bond_xmit_xor(skb, dev);
>+ return bond_3ad_xor_xmit(skb, dev);
> case BOND_MODE_BROADCAST:
> return bond_xmit_broadcast(skb, dev);
>- case BOND_MODE_8023AD:
>- return bond_3ad_xmit_xor(skb, dev);
> case BOND_MODE_ALB:
> return bond_alb_xmit(skb, dev);
> case BOND_MODE_TLB:
>@@ -3861,6 +4020,7 @@ static void bond_uninit(struct net_device *bond_dev)
> struct bonding *bond = netdev_priv(bond_dev);
> struct list_head *iter;
> struct slave *slave;
>+ struct bond_up_slave *arr;
>
> bond_netpoll_cleanup(bond_dev);
>
>@@ -3869,6 +4029,12 @@ static void bond_uninit(struct net_device *bond_dev)
> __bond_release_one(bond_dev, slave->dev, true);
> netdev_info(bond_dev, "Released all slaves\n");
>
>+ arr = rtnl_dereference(bond->slave_arr);
>+ if (arr) {
>+ kfree_rcu(arr, rcu);
>+ RCU_INIT_POINTER(bond->slave_arr, NULL);
>+ }
>+
> list_del(&bond->bond_list);
>
> bond_debug_unregister(bond);
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index 5b022da9cad2..10920f0686e2 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -179,6 +179,12 @@ struct slave {
> struct rtnl_link_stats64 slave_stats;
> };
>
>+struct bond_up_slave {
>+ unsigned int count;
>+ struct rcu_head rcu;
>+ struct slave *arr[0];
>+};
>+
> /*
> * Link pseudo-state only used internally by monitors
> */
>@@ -193,6 +199,7 @@ struct bonding {
> struct slave __rcu *curr_active_slave;
> struct slave __rcu *current_arp_slave;
> struct slave __rcu *primary_slave;
>+ struct bond_up_slave __rcu *slave_arr; /* Array of usable slaves */
> bool force_primary;
> s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
> int (*recv_probe)(const struct sk_buff *, struct bonding *,
>@@ -222,6 +229,7 @@ struct bonding {
> struct delayed_work alb_work;
> struct delayed_work ad_work;
> struct delayed_work mcast_work;
>+ struct delayed_work slave_arr_work;
> #ifdef CONFIG_DEBUG_FS
> /* debugging support via debugfs */
> struct dentry *debug_dir;
>@@ -534,6 +542,8 @@ const char *bond_slave_link_status(s8 link);
> struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
> struct net_device *end_dev,
> int level);
>+int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
>+void bond_slave_arr_work_rearm(struct bonding *bond, unsigned long delay);
>
> #ifdef CONFIG_PROC_FS
> void bond_create_proc_entry(struct bonding *bond);
>--
>2.1.0.rc2.206.gedb03e5
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: [LKP] [net] 4ed2d765dfa: ltp.recv01.4.TFAIL
From: Chuck Ebbert @ 2014-10-01 16:20 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Fengguang Wu, Willem de Bruijn, Dave Hansen, netdev, LKML, lkp
In-Reply-To: <542C2460.8020208@redhat.com>
On Wed, 01 Oct 2014 17:57:20 +0200
Daniel Borkmann <dborkman@redhat.com> wrote:
> On 10/01/2014 05:29 PM, Fengguang Wu wrote:
> > Hi Willem,
> >
> > FYI, we noticed the below LTP failures on commit
> >
> > 4ed2d765dfaccff5ebdac68e2064b59125033a3b ("net-timestamp: TCP timestamping")
>
> I think it already was discussed here:
>
> http://www.spinics.net/lists/netdev/msg297822.html
I've fixed one of those in LTP.
My proposed patch for how to fix the other three didn't get any replies
on the LTP list:
http://sourceforge.net/p/ltp/mailman/message/32866727/
^ permalink raw reply
* [PATCH 1/1 net-next] tcp: add __init to tcp_init_mem
From: Fabian Frederick @ 2014-10-01 16:27 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
tcp_init_mem is only called by __init tcp_init.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/ipv4/tcp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index cf5e508..5c17034 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3172,7 +3172,7 @@ static int __init set_thash_entries(char *str)
}
__setup("thash_entries=", set_thash_entries);
-static void tcp_init_mem(void)
+static void __init tcp_init_mem(void)
{
unsigned long limit = nr_free_buffer_pages() / 8;
limit = max(limit, 128UL);
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net-next] udp: increment UDP_NO_PORTS when dropping unmatched multicasts
From: Rick Jones @ 2014-10-01 16:32 UTC (permalink / raw)
To: David L Stevens, netdev; +Cc: davem
In-Reply-To: <542C23AA.6060505@oracle.com>
On 10/01/2014 08:54 AM, David L Stevens wrote:
> I think this would have the unpleasant side-effect of incrementing a drop
> stat when we have not joined a multicast group that has UDP traffic, but the
> interface is in promiscuous mode. Also, false positives for the multicast
> address filter.
>
> Multicast address filters are not perfect matches, so it is "normal" to receive
> multicasts and broadcasts that are not addressed to our host. I'm not sure those
> should count as "noports" any more than traffic addressed to someone else's IP
> address if we're in promiscuous mode should.
>
> In the multicast case, it would really only make sense if we have actually joined
> the group it's addressed to.
Do you think an "ignored" statistic would be appropriate then?
rick
^ permalink raw reply
* Re: [PATCH net-next 2/2] net/mlx4_core: Disable BF when write combining is not available
From: Alexei Starovoitov @ 2014-10-01 16:52 UTC (permalink / raw)
To: Or Gerlitz
Cc: David S. Miller, netdev@vger.kernel.org, Amir Vadai,
Jack Morgenstein, Moshe Lazer, Tal Alon, Yevgeny Petrilin
In-Reply-To: <1412175282-25212-3-git-send-email-ogerlitz@mellanox.com>
On Wed, Oct 1, 2014 at 7:54 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> From: Moshe Lazer <moshel@mellanox.com>
>
> In mlx4 for better latency, we write send descriptors to a write-combining
> (WC) mapped buffer instead of ringing a doorbell and having the HW fetch
> the descriptor from system memory.
>
> However, if write-combining is not supported on the host, then we
> obtain better latency by using the doorbell-ring/HW fetch mechanism.
>
> The mechanism that uses WC is called Blue-Flame (BF). BF is beneficial
> only when the system supports write combining. When the BF buffer is
> mapped as a write-combine buffer, the HCA receives data in multi-word
> bursts. However, if the BF buffer is mapped only as non-cached, the
> HCA receives data in individual dword chunks, which harms performance.
>
> Therefore, disable blueflame when write combining is not available.
curious, what numbers you're seeing:
- bf=on with wc
- bf=on without wc
- bf=off and doorbell
they will help to justify this change.
^ permalink raw reply
* Re: [PATCH net-next] udp: increment UDP_NO_PORTS when dropping unmatched multicasts
From: David L Stevens @ 2014-10-01 16:59 UTC (permalink / raw)
To: Rick Jones, netdev; +Cc: davem
In-Reply-To: <542C2CAB.2030501@hp.com>
On 10/01/2014 12:32 PM, Rick Jones wrote:
>
> Do you think an "ignored" statistic would be appropriate then?
I guess I don't know what that means. If we didn't join the group, it
should be treated like anything we'd receive in promiscuous mode that
is not addressed to us. I'd assume interface stats are going up, but not
any protocol stats in that case. (already the case)
If we did join the group, it should be treated the way we would, say,
a broadcast UDP packet where we have no listener on that port.
It isn't clear to me that maintaining the stat is worth checking for
group membership, but we wouldn't want an admin to conclude there is
a problem or an error because we increment a stat when doing the
equivalent of hardware MAC address filtering, just because multicast
address filters are defined to be leaky.
+-DLS
^ permalink raw reply
* Re: [PATCH V1 net-next 2/4] net/mlx5_core: Use hardware registers description header file
From: Alexei Starovoitov @ 2014-10-01 17:12 UTC (permalink / raw)
To: Eli Cohen
Cc: David S. Miller, netdev@vger.kernel.org, Or Gerlitz,
Yevgeny Petrilin, Joe Perches
In-Reply-To: <1412169488-17500-3-git-send-email-eli@mellanox.com>
On Wed, Oct 1, 2014 at 6:18 AM, Eli Cohen <eli@mellanox.com> wrote:
>
> Change-Id: Ieea94e4b329ec46902b1634cf66b0c3a04445a04
checkpatch errors on gerrit tags by default,
but I'm not sure what's an official stance on it, since I see patches
with this tag slip through here and there.
> +static inline void non_existent_function(void)
> +{
> + pr_info("%s\n", __func__);
> +}
> +
> +static inline void non_existent_function_sz_align32(void)
> +{
> + pr_info("%s\n", __func__);
> +}
> +
> +static inline void non_existent_function_const_overflow(void)
> +{
> + pr_info("%s\n", __func__);
> +}
> +
> +static inline void memcpy_cpu_to_be32(void *dst, void *src, int len)
> +{
> + u32 *dst_u32 = (u32 *)dst;
> + int i;
> +
> + memcpy(dst, src, len);
> +
> + for (i = 0; i < (len >> 2); i++)
> + dst_u32[i] = cpu_to_be32(dst_u32[i]);
it just assumes that pointers are aligned and len is power of 4 ?
> +}
> +
> +/* insert a value to a struct */
> +#define MLX5_SET(typ, p, fld, v) do { \
> + if (__mlx5_st_sz_bits(typ) % 32) \
> + non_existent_function_sz_align32(); \
> + *((__be32 *)(p) + __mlx5_dw_off(typ, fld)) = \
> + cpu_to_be32((be32_to_cpu(*((__be32 *)(p) + __mlx5_dw_off(typ, fld))) & \
> + (~__mlx5_dw_mask(typ, fld))) | (((v) & __mlx5_mask(typ, fld)) \
> + << __mlx5_dw_bit_off(typ, fld))); \
> +} while (0)
> +
> +#define MLX5_GET(typ, p, fld) ((be32_to_cpu(*((__be32 *)(p) +\
> +__mlx5_dw_off(typ, fld))) >> __mlx5_dw_bit_off(typ, fld)) & \
> +__mlx5_mask(typ, fld))
> +
> +#define MLX5_GET_PR(typ, p, fld) ({ \
> + u32 ___t = MLX5_GET(typ, p, fld); \
> + pr_debug(#fld " = 0x%x\n", ___t); \
> + ___t; \
> +})
> +
> +#define MLX5_SET64(typ, p, fld, v) do { \
> + if (__mlx5_bit_sz(typ, fld) != 64) \
> + non_existent_function(); \
> + else if (__mlx5_bit_off(typ, fld) % 64) \
> + non_existent_function(); \
> + else \
> + *((__be64 *)(p) + __mlx5_64_off(typ, fld)) = cpu_to_be64(v); \
> +} while (0)
is it possible to use BUILD_BUG_ON instead of runtime pr_info() ?
^ permalink raw reply
* Re: [PATCH net-next] udp: increment UDP_NO_PORTS when dropping unmatched multicasts
From: Sergei Shtylyov @ 2014-10-01 17:14 UTC (permalink / raw)
To: Rick Jones, netdev; +Cc: davem
In-Reply-To: <20141001151921.7131E29003A2@tardy>
Hello.
On 10/01/2014 07:19 PM, Rick Jones wrote:
> From: Rick Jones <rick.jones2@hp.com>
> When there is absolutely no match for an incoming UDP multicast we
> should increment a suitable UDP statistic in addition to the kfree_skb().
> When there were some matches, we should not and simply call consume_skb().
> Signed-off-by: Rick Jones <rick.jones2@hp.com>
> ---
> Noticed __udp4_lib_mcast_deliver showing-up in a perf dropped packet
> profile on a system sitting on a network with a bunch of Windows boxes
> sending what they are fond of sending.
> Verified that UDP_MIB_NOPORTS increments when it was not before and hit
> the system with a bit of netperf multicast UDP_RR but that is the extent
> of the testing performed.
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index cd0db54..376e3d3 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1656,6 +1656,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
> int dif = skb->dev->ifindex;
> unsigned int count = 0, offset = offsetof(typeof(*sk), sk_nulls_node);
> unsigned int hash2 = 0, hash2_any = 0, use_hash2 = (hslot->count > 10);
> + unsigned int inner_flushed = 0;
>
> if (use_hash2) {
> hash2_any = udp4_portaddr_hash(net, htonl(INADDR_ANY), hnum) &
> @@ -1694,8 +1695,12 @@ start_lookup:
> */
> if (count) {
> flush_stack(stack, count, skb, count - 1);
> - } else {
> + } else if (!inner_flushed) {
I don't see where this new variable is changed. It's always 0 here.
> + UDP_INC_STATS_BH(net, UDP_MIB_NOPORTS, 0);
> kfree_skb(skb);
> + } else {
> + /* there were matches flushed in the for_each */
> + consume_skb(skb);
> }
> return 0;
> }
WBR, Sergei
^ permalink raw reply
* Re: [PATCH v2 net-next 01/10] r8169:change uppercase number to lowercase nubmer
From: Sergei Shtylyov @ 2014-10-01 17:17 UTC (permalink / raw)
To: Chun-Hao Lin, netdev; +Cc: nic_swsd, linux-kernel
In-Reply-To: <1412176641-24393-1-git-send-email-hau@realtek.com>
Hello.
On 10/01/2014 07:17 PM, Chun-Hao Lin wrote:
> Signed-off-by: Chun-Hao Lin <hau@realtek.com>
How about correcting typo in the subject: s/nubmer/number/?
WBR, Sergei
^ permalink raw reply
* [PATCH 1/1 net-next] inet: frags: add __init to ip4_frags_ctl_register
From: Fabian Frederick @ 2014-10-01 17:18 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
ip4_frags_ctl_register is only called by __init ipfrag_init
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/ipv4/ip_fragment.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 15f0e2b..2811cc1 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -790,7 +790,7 @@ static void __net_exit ip4_frags_ns_ctl_unregister(struct net *net)
kfree(table);
}
-static void ip4_frags_ctl_register(void)
+static void __init ip4_frags_ctl_register(void)
{
register_net_sysctl(&init_net, "net/ipv4", ip4_frags_ctl_table);
}
@@ -804,7 +804,7 @@ static inline void ip4_frags_ns_ctl_unregister(struct net *net)
{
}
-static inline void ip4_frags_ctl_register(void)
+static inline void __init ip4_frags_ctl_register(void)
{
}
#endif
--
1.9.3
^ permalink raw reply related
* Re: [PATCH V1 net-next 2/4] net/mlx5_core: Use hardware registers description header file
From: Sergei Shtylyov @ 2014-10-01 17:21 UTC (permalink / raw)
To: Alexei Starovoitov, Eli Cohen
Cc: David S. Miller, netdev@vger.kernel.org, Or Gerlitz,
Yevgeny Petrilin, Joe Perches
In-Reply-To: <CAADnVQ+0q=j9soWX-ge6Re4F6sDahq=O-Ptaguca8WUaTE-qQw@mail.gmail.com>
Hello.
On 10/01/2014 09:12 PM, Alexei Starovoitov wrote:
>> Change-Id: Ieea94e4b329ec46902b1634cf66b0c3a04445a04
> checkpatch errors on gerrit tags by default,
> but I'm not sure what's an official stance on it, since I see patches
> with this tag slip through here and there.
It's allowed by DaveM.
WBR, Sergei
^ permalink raw reply
* Re: Why include/net/tcp_states.h is not exported to user-space?
From: Arnaldo Carvalho de Melo @ 2014-10-01 17:25 UTC (permalink / raw)
To: Cong Wang; +Cc: David Miller, netdev
In-Reply-To: <CAHA+R7MDGKA1Z-1ytk8-+9XaKNcW+kG7UBn8drhdbLJeQYBCyQ@mail.gmail.com>
Em Tue, Sep 30, 2014 at 11:12:04AM -0700, Cong Wang escreveu:
> These states are needed for user-space to specify ->idiag_states.
> Currently both libnl and ss make their own copy of these definitions.
> Of course, new sock diag is not just limited to TCP, so we probably
> need to rename them to SS_* before exporting them.
>
> I can send a patch if you agree.
One would have to check what are the current best practices for doing
that, i.e. probably this would have to go via
include/uapi//linux/tcp_states.h? Or take the time and create a new
header for the inet_diag.h stuff as this is not TCP specific for quite a
while?
If you could investigate how to properly expose this and then come back
with a patch proposal I think that would ease reviewing :-)
- Arnaldo
^ permalink raw reply
* [PATCH v2] ssb: Fix Sparse error in main
From: Pramod Gurav @ 2014-10-01 17:28 UTC (permalink / raw)
To: linux-kernel; +Cc: Pramod Gurav, Michael Buesch, netdev
This change fixes below sparse error:
drivers/ssb/main.c:94:16: warning: symbol 'ssb_sdio_func_to_bus'
was not declared. Should it be static?
Cc: Michael Buesch <m@bues.ch>
Cc: netdev@vger.kernel.org
Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
---
Changes since v1:
Removed the function as it is not called anywhere in the kernel
as per suggestion from Michael Buesch.
drivers/ssb/main.c | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c
index 2fead38..1e180c4 100644
--- a/drivers/ssb/main.c
+++ b/drivers/ssb/main.c
@@ -90,25 +90,6 @@ found:
}
#endif /* CONFIG_SSB_PCMCIAHOST */
-#ifdef CONFIG_SSB_SDIOHOST
-struct ssb_bus *ssb_sdio_func_to_bus(struct sdio_func *func)
-{
- struct ssb_bus *bus;
-
- ssb_buses_lock();
- list_for_each_entry(bus, &buses, list) {
- if (bus->bustype == SSB_BUSTYPE_SDIO &&
- bus->host_sdio == func)
- goto found;
- }
- bus = NULL;
-found:
- ssb_buses_unlock();
-
- return bus;
-}
-#endif /* CONFIG_SSB_SDIOHOST */
-
int ssb_for_each_bus_call(unsigned long data,
int (*func)(struct ssb_bus *bus, unsigned long data))
{
--
1.8.3.2
^ permalink raw reply related
* Re: [net-next PATCH V5] qdisc: bulk dequeue support for qdiscs with TCQ_F_ONETXQUEUE
From: Jesper Dangaard Brouer @ 2014-10-01 17:28 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Tom Herbert, David Miller, Linux Netdev List, Eric Dumazet,
Hannes Frederic Sowa, Florian Westphal, Daniel Borkmann,
Alexander Duyck, John Fastabend, Dave Taht,
Toke Høiland-Jørgensen, brouer
In-Reply-To: <542C1F1F.90404@mojatatu.com>
On Wed, 01 Oct 2014 11:34:55 -0400 Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> The issue is: if i am going to attempt to do a bulk transfer
> every single time (with new code) and for the common use case the
> result is "no need to do bulking" then you just added extra code that
> is unnecessary for that common case.
> Even a single extra if statement at high packet rate is still
> costly and would be easy to observe.
[...]
>
> That is if the added code ends up being hit
> meaningfully. Jesper said (and it was my experience as well)
> that it was _hard_ to achieve bulking in such a case.
Let me close this one...
The bulk dequeue code *is* being hit meaningfully. It is _only_ going
to be activated when (q->qlen > 0) see __dev_xmit_skb() in dev.c.
When there is no queue pfifo_fast is allowed to directly call
sch_direct_xmit() without a enqueue/dequeue cycle. Thus, this code
would not be hit for every packet.
Thus, code is activated only when q->qlen is >= 1. And I have already
shown that we see a win with just bulking 2 packets:
"Subject: qdisc/UDP_STREAM: measuring effect of qdisc bulk dequeue"
http://thread.gmane.org/gmane.linux.network/331152/focus=331154
I actually believe, that I have already measured and shown that going
though the bulk dequeue is a win, this should show direct_xmit vs.
bulking:
"Subject: qdisc/trafgen: Measuring effect of qdisc bulk dequeue, with trafgen"
http://thread.gmane.org/gmane.linux.network/331152
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH 1/1 net-next] cipso: add __init to cipso_v4_cache_init
From: Fabian Frederick @ 2014-10-01 17:30 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
cipso_v4_cache_init is only called by __init cipso_v4_init
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/ipv4/cipso_ipv4.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 05b708b..4715f25 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -246,7 +246,7 @@ static u32 cipso_v4_map_cache_hash(const unsigned char *key, u32 key_len)
* success, negative values on error.
*
*/
-static int cipso_v4_cache_init(void)
+static int __init cipso_v4_cache_init(void)
{
u32 iter;
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net-next] udp: increment UDP_NO_PORTS when dropping unmatched multicasts
From: Rick Jones @ 2014-10-01 17:32 UTC (permalink / raw)
To: David L Stevens, netdev; +Cc: davem
In-Reply-To: <542C32E6.7080106@oracle.com>
On 10/01/2014 09:59 AM, David L Stevens wrote:
> On 10/01/2014 12:32 PM, Rick Jones wrote:
>
>>
>> Do you think an "ignored" statistic would be appropriate then?
>
> I guess I don't know what that means.
It would be an added statistic for "ignored" UDP multicast datagrams,
incremented instead of UDP_MIB_NOPORTS. "UDP_MIB_IGNOREDMULTI" if you
will.
Similar in concept to what HP-UX NIC drivers would increment when they
received a frame for which there was no bound protocol - "inbound
unknown protocols" but not a drop
http://ptgmedia.pearsoncmg.com/images/chap1_0130428167/elementLinks/01fig16.gif
rick
^ permalink raw reply
* Re: linux-next: build failure after merge of the net-next tree
From: David Miller @ 2014-10-01 17:42 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, alexander.h.duyck,
jeffrey.t.kirsher
In-Reply-To: <20141001170403.7a319458@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 1 Oct 2014 17:04:03 +1000
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 1 Oct 2014 17:00:49 +1000
> Subject: [PATCH] fm10k: using vmalloc requires including linux/vmalloc.h
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Applied, thanks Stephen.
^ permalink raw reply
* Re: [PATCH net-next] udp: increment UDP_NO_PORTS when dropping unmatched multicasts
From: David L Stevens @ 2014-10-01 17:43 UTC (permalink / raw)
To: Rick Jones, netdev; +Cc: davem
In-Reply-To: <542C3A95.8050804@hp.com>
On 10/01/2014 01:32 PM, Rick Jones wrote:
> It would be an added statistic for "ignored" UDP multicast datagrams, incremented instead of UDP_MIB_NOPORTS. "UDP_MIB_IGNOREDMULTI" if you will.
>
> Similar in concept to what HP-UX NIC drivers would increment when they received a frame for which there was no bound protocol - "inbound unknown protocols" but not a drop
> http://ptgmedia.pearsoncmg.com/images/chap1_0130428167/elementLinks/01fig16.gif
I guess I'm ok with that.
Ideally, it wouldn't be affected by running a sniffer, so I think best would be to increment
NOPORTS when someone has joined the group on the interface and IGNOREDMULTI when nobody has,
but I'm not sure it's worth the trouble to check. So I'm good with that, or IGNOREDMULTI all
the time.
+-DLS
^ permalink raw reply
* Re: [PATCH net-next] ipv4: mentions skb_gro_postpull_rcsum() in inet_gro_receive()
From: David Miller @ 2014-10-01 17:44 UTC (permalink / raw)
To: eric.dumazet; +Cc: Yuval.Mintz, therbert, netdev
In-Reply-To: <1412140325.16704.31.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 30 Sep 2014 22:12:05 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> Proper CHECKSUM_COMPLETE support needs to adjust skb->csum
> when we remove one header. Its done using skb_gro_postpull_rcsum()
>
> In the case of IPv4, we know that the adjustment is not really needed,
> because the checksum over IPv4 header is 0. Lets add a comment to
> ease code comprehension and avoid copy/paste errors.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH] net: ethernet : stmicro: fixed power suspend and resume failure in stmmac driver
From: David Miller @ 2014-10-01 17:45 UTC (permalink / raw)
To: hliang1025; +Cc: peppe.cavallaro, netdev, linux-kernel, adi-linux-docs
In-Reply-To: <CAPig_t=oA6BuAt77DDsLqrtRBDbV_vsoAxM31XxLjkAFpYG1-Q@mail.gmail.com>
From: Hao Liang <hliang1025@gmail.com>
Date: Wed, 1 Oct 2014 14:08:28 +0800
> I double-check my patch and the ->mac->xxx calls are still under the lock.
> I think that lock is trying to protect priv struct and related data, so i
> just remove some functions have no bearing on priv struct.
It's preventing parallel invocations of the ->mac->xxx calls.
The other instances are in device open/close, where RTNL semaphore is
held, and no other code paths in the driver can be active.
You need the lock.
^ permalink raw reply
* Re: [PATCH net-next] udp: increment UDP_NO_PORTS when dropping unmatched multicasts
From: Rick Jones @ 2014-10-01 17:51 UTC (permalink / raw)
To: eric.dumazet, netdev; +Cc: davem
In-Reply-To: <542C3D28.6050705@oracle.com>
On 10/01/2014 10:43 AM, David L Stevens wrote:
>
>
> On 10/01/2014 01:32 PM, Rick Jones wrote:
>
>> It would be an added statistic for "ignored" UDP multicast datagrams, incremented instead of UDP_MIB_NOPORTS. "UDP_MIB_IGNOREDMULTI" if you will.
>>
>> Similar in concept to what HP-UX NIC drivers would increment when they received a frame for which there was no bound protocol - "inbound unknown protocols" but not a drop
>> http://ptgmedia.pearsoncmg.com/images/chap1_0130428167/elementLinks/01fig16.gif
>
> I guess I'm ok with that.
>
> Ideally, it wouldn't be affected by running a sniffer, so I think best would be to increment
> NOPORTS when someone has joined the group on the interface and IGNOREDMULTI when nobody has,
> but I'm not sure it's worth the trouble to check. So I'm good with that, or IGNOREDMULTI all
> the time.
Eric -
What is your feeling? I have a UDP_MIB_NOPORTS v2 with UDP lite, fixed
inner_flushed and IPv6 I can post now, or I can tweak it to add an
IGNOREDMULTI stat and use that instead of NOPORTS. If taking the
IGNOREDMULTI path, I'd be inclined to go with consume_skb()
unconditionally since the philosophy would be that it is an ignored
packet rather than a drop (similar to the recent change in ARP).
rick
^ permalink raw reply
* Re: [PATCH] net: dsa: Fix build warning for !PM_SLEEP
From: Florian Fainelli @ 2014-10-01 18:13 UTC (permalink / raw)
To: Thierry Reding, David S. Miller; +Cc: netdev, linux-kernel
In-Reply-To: <1412164740-32452-1-git-send-email-thierry.reding@gmail.com>
On 10/01/2014 04:59 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The dsa_switch_suspend() and dsa_switch_resume() functions are only used
> when PM_SLEEP is enabled, so they need #ifdef CONFIG_PM_SLEEP protection
> to avoid a compiler warning.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Thanks Thierry!
> ---
> net/dsa/dsa.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 6905f2d84c44..22f34cf4cb27 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -238,6 +238,7 @@ static void dsa_switch_destroy(struct dsa_switch *ds)
> {
> }
>
> +#ifdef CONFIG_PM_SLEEP
> static int dsa_switch_suspend(struct dsa_switch *ds)
> {
> int i, ret = 0;
> @@ -280,6 +281,7 @@ static int dsa_switch_resume(struct dsa_switch *ds)
>
> return 0;
> }
> +#endif
>
>
> /* link polling *************************************************************/
>
^ permalink raw reply
* Re: [PATCH net v6 4/4] tg3: Fix tx_pending checks for tg3_tso_bug
From: Prashant @ 2014-10-01 18:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, bpoirier, mchan, netdev, linux-kernel
In-Reply-To: <1412137489.16704.26.camel@edumazet-glaptop2.roam.corp.google.com>
On 9/30/2014 9:24 PM, Eric Dumazet wrote:
> On Tue, 2014-09-30 at 20:14 -0700, Prashant wrote:
>
>> Sorry about the late reply, out of all the HW bug conditions checked in
>> tg3_tx_frag_set() the most frequently hit condition is the short 8 byte
>> dma bug, where the chip cannot handle TX descriptors whose data buffer
>> is 8 bytes or less. Most of the LSO skb's given to the driver has their
>> fragments filled upto PAGE_SIZE (expect the last fragment depending on
>> skb->len). And if such a LSO skb's last fragment meets the 8 bytes HW
>> bug condition the above routine will not help workaround this particular
>> case.
>
> Thats pretty easy to work around.
>
> Say rebuilt skb has N frags (N > 1 given your description)
>
> They are numbered 0, ... N-2, N-1
>
> Instead of filling N-2 completely, fill it to PAGE_SIZE-8, so that last
> frag has at least 8 bytes in it.
definitely it can be tweaked to match what is needed with additional
workarounds.
>
> Also take a look at commit 2e4e44107176d552f8bb1bb76053e850e3809841
> ("net: add alloc_skb_with_frags() helper")
>
>
This helper is much modular/flexible than the initial proposed one. Thanks.
>
^ permalink raw reply
* [PATCH net-next] net: bcmgenet: fix bcmgenet_put_tx_csum()
From: Petri Gynther @ 2014-10-01 18:30 UTC (permalink / raw)
To: netdev; +Cc: davem, f.fainelli
bcmgenet_put_tx_csum() needs to return skb pointer back to the caller
because it reallocates a new one in case of lack of skb headroom.
Signed-off-by: Petri Gynther <pgynther@google.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 77cb755..d51729c 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1054,7 +1054,8 @@ static int bcmgenet_xmit_frag(struct net_device *dev,
/* Reallocate the SKB to put enough headroom in front of it and insert
* the transmit checksum offsets in the descriptors
*/
-static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
+static struct sk_buff *bcmgenet_put_tx_csum(struct net_device *dev,
+ struct sk_buff *skb)
{
struct status_64 *status = NULL;
struct sk_buff *new_skb;
@@ -1072,7 +1073,7 @@ static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
if (!new_skb) {
dev->stats.tx_errors++;
dev->stats.tx_dropped++;
- return -ENOMEM;
+ return NULL;
}
skb = new_skb;
}
@@ -1090,7 +1091,7 @@ static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
ip_proto = ipv6_hdr(skb)->nexthdr;
break;
default:
- return 0;
+ return skb;
}
offset = skb_checksum_start_offset(skb) - sizeof(*status);
@@ -1111,7 +1112,7 @@ static int bcmgenet_put_tx_csum(struct net_device *dev, struct sk_buff *skb)
status->tx_csum_info = tx_csum_info;
}
- return 0;
+ return skb;
}
static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -1158,8 +1159,8 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
/* set the SKB transmit checksum */
if (priv->desc_64b_en) {
- ret = bcmgenet_put_tx_csum(dev, skb);
- if (ret) {
+ skb = bcmgenet_put_tx_csum(dev, skb);
+ if (!skb) {
ret = NETDEV_TX_OK;
goto out;
}
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox