* Re: [patch net-next 0/2] team: two RCU fixups
From: David Miller @ 2012-06-20 21:05 UTC (permalink / raw)
To: jpirko; +Cc: netdev, eric.dumazet, jbrouer, paulmck, wfg
In-Reply-To: <1340206321-5986-1-git-send-email-jpirko@redhat.com>
From: Jiri Pirko <jpirko@redhat.com>
Date: Wed, 20 Jun 2012 17:31:59 +0200
> Jiri Pirko (2):
> team: use rcu_access_pointer to access RCU pointer by writer
> team: use RCU_INIT_POINTER for NULL assignment of RCU pointer
Applied, but this makes your subsequent patch not apply.
^ permalink raw reply
* Re: [PATCH v3] ipv4: Early TCP socket demux.
From: Julian Anastasov @ 2012-06-20 21:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20120620.023002.243497856926894946.davem@davemloft.net>
Hello,
On Wed, 20 Jun 2012, David Miller wrote:
> > Date: Wed, 20 Jun 2012 10:00:37 +0300 (EEST)
> >
> >> if (skb->dev != dst->dev)
> >> dst = NULL;
> >
> > That makes the most sense.
>
> Doesn't work, dst->dev is &net->loopback_dev for these locally
> destined input routes.
I see, correct.
> We have to instead check rt->rt_iif or similar.
Yes, rt_iif should be valid for packets with
skb->dst = NULL. It is incorrect only on loopback
traffic diverted to "lo", i.e. when skb->dst != NULL.
But it concerns UDP which is not handled by GRO yet.
When UDP support for GRO is implemented
dev_gro_receive() should additionally check skb_dst
to ignore local copy of b-m/cast traffic sent via
ip_mc_output -> ip_dev_loopback_xmit because
in this case dst->dev and skb->dst can be eth0
where NETIF_F_GRO can be set.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [patch net-next] team: do RCU update path fixups
From: David Miller @ 2012-06-20 21:05 UTC (permalink / raw)
To: eric.dumazet; +Cc: jpirko, netdev, jbrouer, paulmck, wfg
In-Reply-To: <1340219643.4604.1641.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 20 Jun 2012 21:14:03 +0200
> On Wed, 2012-06-20 at 20:39 +0200, Jiri Pirko wrote:
>> Use rcu_access_pointer and rcu_dereference_protected
>> to access RCU pointer by updater.
>> Use RCU_INIT_POINTER for NULL assignment of RCU pointer.
>>
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>> ---
>> drivers/net/team/team_mode_activebackup.c | 8 ++++++--
>> drivers/net/team/team_mode_loadbalance.c | 14 ++++++++++----
>> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> Seems good to me, thanks.
>
> Acked-by: Eric Dumazet <edumazet@google.com>
This patch doesn't apply after the 2 patch set you sent right
before this one.
^ permalink raw reply
* Re: [PATCH v2] ipv4: Early TCP socket demux.
From: Stephen Hemminger @ 2012-06-20 21:04 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, bhutchings, netdev
In-Reply-To: <20120620.140121.1603737472432326278.davem@davemloft.net>
On Wed, 20 Jun 2012 14:01:21 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 20 Jun 2012 20:40:04 +0200
>
> > If someone wants to tune its linux router, he probably already disables
> > GRO because of various issues with too big packets.
> >
> > GRO adds a significant cost to forwarding path.
>
> No, Ben is right Eric. GRO decreases the costs, because it means we
> only need to make one forwarding/netfilter/classification decision for
> N packets instead of 1.
GRO is also important for routers that interact with VM's.
It helps reduce the per-packet wakeup of the guest VM's.
^ permalink raw reply
* Re: [PATCH v2] ipv4: Early TCP socket demux.
From: David Miller @ 2012-06-20 21:01 UTC (permalink / raw)
To: eric.dumazet; +Cc: bhutchings, shemminger, netdev
In-Reply-To: <1340217604.4604.1569.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 20 Jun 2012 20:40:04 +0200
> If someone wants to tune its linux router, he probably already disables
> GRO because of various issues with too big packets.
>
> GRO adds a significant cost to forwarding path.
No, Ben is right Eric. GRO decreases the costs, because it means we
only need to make one forwarding/netfilter/classification decision for
N packets instead of 1.
^ permalink raw reply
* Re: [PATCH v2] can: c_can_pci: fix compilation on non HAVE_CLK archs
From: David Miller @ 2012-06-20 20:56 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can, federico.vaga
In-Reply-To: <1340208266-22098-1-git-send-email-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Wed, 20 Jun 2012 18:04:26 +0200
> In commit:
>
> 5b92da0 c_can_pci: generic module for C_CAN/D_CAN on PCI
>
> the c_can_pci driver has been added. It uses clk_*() functions
> resulting in a link error on archs without clock support. This
> patch removed these clk_() functions as these parts of the driver
> are not tested.
>
> Cc: Federico Vaga <federico.vaga@gmail.com>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Applied.
^ permalink raw reply
* Re: [RFC net-next 00/14] default maximal number of RSS queues in mq drivers
From: Ben Hutchings @ 2012-06-20 20:48 UTC (permalink / raw)
To: Yuval Mintz
Cc: netdev, davem, eilong, Divy Le Ray, Or Gerlitz, Jon Mason,
Anirban Chakraborty, Jitendra Kalsaria, Ron Mercer, Jeff Kirsher,
Jon Mason, Andrew Gallatin, Sathya Perla, Subbu Seetharaman,
Ajit Khaparde, Matt Carlson, Michael Chan
In-Reply-To: <1340225015.2576.27.camel@bwh-desktop.uk.solarflarecom.com>
Also, I would recommend encapsulating the calculation of default number
of RSS queues in a function, rather than repeating it in every driver.
That will make it easier to replace with something more sophisticated
and configurable later on.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [RFC net-next 00/14] default maximal number of RSS queues in mq drivers
From: Ben Hutchings @ 2012-06-20 20:43 UTC (permalink / raw)
To: Yuval Mintz
Cc: netdev, davem, eilong, Divy Le Ray, Or Gerlitz, Jon Mason,
Anirban Chakraborty, Jitendra Kalsaria, Ron Mercer, Jeff Kirsher,
Jon Mason, Andrew Gallatin, Sathya Perla, Subbu Seetharaman,
Ajit Khaparde, Matt Carlson, Michael Chan
In-Reply-To: <1340118848-30978-1-git-send-email-yuvalmin@broadcom.com>
On Tue, 2012-06-19 at 18:13 +0300, Yuval Mintz wrote:
> Different vendors support different number of RSS queues by default. Today,
> there exists an ethtool API through which users can change the number of
> channels their driver supports; This enables us to pursue the goal of using
> a default number of RSS queues in various multi-queue drivers.
>
> This RFC intendeds to achieve the above default, by upper-limiting the number
> of interrupts multi-queue drivers request (by default, not via the new API)
> with correlation to the number of cpus on the machine.
>
> After examining multi-queue drivers that call alloc_etherdev_mq[s],
> it became evident that most drivers allocate their devices using hard-coded
> values. Changing those defaults directly will most likely cause a regression.
>
> However, (most) multi-queue driver look at the number of online cpus when
> requesting for interrupts. We assume that the number of interrupts the
> driver manages to request is propagated across the driver, and the number
> of RSS queues it configures is based upon it.
>
> This RFC modifies said logic - if the number of cpus is large enough, use
> a smaller default value instead. This serves 2 main purposes:
> 1. A step forward unity in the number of RSS queues of various drivers.
> 2. It prevents wasteful requests for interrupts on machines with many cpus.
[...]
> Driver identified as multi-queue, no reference to number of online cpus found,
> and thus unhandled in this RFC:
[...]
> * sfc efx
[...]
In sfc we currently look at the CPU topology to count cores instead of
threads. The result is the same unless the system has hyperthreading
(or other SMT) enabled.
I've seen many diagnostic reports from customer support tickets where
there were 32 queue-sets and MSI-X vectors in use (the maximum currently
supported by the driver), but very few had a problem with that.
I would be interested in a scheme to use fewer queues for RSS but more
for flow steering (accelerated RFS, XPS and ethtool NFC). We had some
discussion of this at last year's netconf but sadly I've not yet found
time to work on it.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH][RESEND] bonding: delete migrated IP addresses from the rlb hash table
From: Jiri Bohac @ 2012-06-20 20:37 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, netdev
Hi, this is a resend of the patch discussed here:
http://thread.gmane.org/gmane.linux.network/228076
It has been updated to apply to the lastest net-next.
Bonding in balance-alb mode records information from ARP packets
passing through the bond in a hash table (rx_hashtbl).
At certain situations (e.g. link change of a slave),
rlb_update_rx_clients() will send out ARP packets to update ARP
caches of other hosts on the network to achieve RX load
balancing.
The problem is that once an IP address is recorded in the hash
table, it stays there indefinitely. If this IP address is
migrated to a different host in the network, bonding still sends
out ARP packets that poison other systems' ARP caches with
invalid information.
This patch solves this by looking at all incoming ARP packets,
and checking if the source IP address is one of the source
addresses stored in the rx_hashtbl. If it is, but the MAC
addresses differ, the corresponding hash table entries are
removed. Thus, when an IP address is migrated, the first ARP
broadcast by its new owner will purge the offending entries of
rx_hashtbl.
The hash table is hashed by ip_dst. To be able to do the above
check efficiently (not walking the whole hash table), we need a
reverse mapping (by ip_src).
I added three new members in struct rlb_client_info:
rx_hashtbl[x].src_first will point to the start of a list of
entries for which hash(ip_src) == x.
The list is linked with src_next and src_prev.
When an incoming ARP packet arrives at rlb_arp_recv()
rlb_purge_src_ip() can quickly walk only the entries on the
corresponding lists, i.e. the entries that are likely to contain
the offending IP address.
To avoid confusion, I renamed these existing fields of struct
rlb_client_info:
next -> used_next
prev -> used_prev
rx_hashtbl_head -> rx_hashtbl_used_head
(The current linked list is _not_ a list of hash table
entries with colliding ip_dst. It's a list of entries that are
being used; its purpose is to avoid walking the whole hash table
when looking for used entries.)
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index e15cc11..8505a24 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -84,6 +84,9 @@ static inline struct arp_pkt *arp_pkt(const struct sk_buff *skb)
/* Forward declaration */
static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[]);
+static void rlb_purge_src_ip(struct bonding *bond, struct arp_pkt *arp);
+static void rlb_src_unlink(struct bonding *bond, u32 index);
+static void rlb_src_link(struct bonding *bond, u32 ip_src_hash, u32 ip_dst_hash);
static inline u8 _simple_hash(const u8 *hash_start, int hash_size)
{
@@ -354,6 +357,17 @@ static int rlb_arp_recv(const struct sk_buff *skb, struct bonding *bond,
if (!arp)
goto out;
+ /* We received an ARP from arp->ip_src.
+ * We might have used this IP address previously (on the bonding host
+ * itself or on a system that is bridged together with the bond).
+ * However, if arp->mac_src is different than what is stored in
+ * rx_hashtbl, some other host is now using the IP and we must prevent
+ * sending out client updates with this IP address and the old MAC address.
+ * Clean up all hash table entries that have this address as ip_src but
+ * have a dirrerent mac_src.
+ */
+ rlb_purge_src_ip(bond, arp);
+
if (arp->op_code == htons(ARPOP_REPLY)) {
/* update rx hash table for this ARP */
rlb_update_entry_from_arp(bond, arp);
@@ -432,9 +446,9 @@ static void rlb_clear_slave(struct bonding *bond, struct slave *slave)
_lock_rx_hashtbl_bh(bond);
rx_hash_table = bond_info->rx_hashtbl;
- index = bond_info->rx_hashtbl_head;
+ index = bond_info->rx_hashtbl_used_head;
for (; index != RLB_NULL_INDEX; index = next_index) {
- next_index = rx_hash_table[index].next;
+ next_index = rx_hash_table[index].used_next;
if (rx_hash_table[index].slave == slave) {
struct slave *assigned_slave = rlb_next_rx_slave(bond);
@@ -519,8 +533,8 @@ static void rlb_update_rx_clients(struct bonding *bond)
_lock_rx_hashtbl_bh(bond);
- hash_index = bond_info->rx_hashtbl_head;
- for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->next) {
+ hash_index = bond_info->rx_hashtbl_used_head;
+ for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->used_next) {
client_info = &(bond_info->rx_hashtbl[hash_index]);
if (client_info->ntt) {
rlb_update_client(client_info);
@@ -548,8 +562,8 @@ static void rlb_req_update_slave_clients(struct bonding *bond, struct slave *sla
_lock_rx_hashtbl_bh(bond);
- hash_index = bond_info->rx_hashtbl_head;
- for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->next) {
+ hash_index = bond_info->rx_hashtbl_used_head;
+ for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->used_next) {
client_info = &(bond_info->rx_hashtbl[hash_index]);
if ((client_info->slave == slave) &&
@@ -578,8 +592,8 @@ static void rlb_req_update_subnet_clients(struct bonding *bond, __be32 src_ip)
_lock_rx_hashtbl(bond);
- hash_index = bond_info->rx_hashtbl_head;
- for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->next) {
+ hash_index = bond_info->rx_hashtbl_used_head;
+ for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->used_next) {
client_info = &(bond_info->rx_hashtbl[hash_index]);
if (!client_info->slave) {
@@ -625,6 +639,7 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
/* update mac address from arp */
memcpy(client_info->mac_dst, arp->mac_dst, ETH_ALEN);
}
+ memcpy(client_info->mac_src, arp->mac_src, ETH_ALEN);
assigned_slave = client_info->slave;
if (assigned_slave) {
@@ -647,6 +662,13 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
assigned_slave = rlb_next_rx_slave(bond);
if (assigned_slave) {
+ if (!(client_info->assigned && client_info->ip_src == arp->ip_src)) {
+ /* ip_src is going to be updated, fix the src hash list */
+ u32 hash_src = _simple_hash((u8 *)&arp->ip_src, sizeof(arp->ip_src));
+ rlb_src_unlink(bond, hash_index);
+ rlb_src_link(bond, hash_src, hash_index);
+ }
+
client_info->ip_src = arp->ip_src;
client_info->ip_dst = arp->ip_dst;
/* arp->mac_dst is broadcast for arp reqeusts.
@@ -654,6 +676,7 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
* upon receiving an arp reply.
*/
memcpy(client_info->mac_dst, arp->mac_dst, ETH_ALEN);
+ memcpy(client_info->mac_src, arp->mac_src, ETH_ALEN);
client_info->slave = assigned_slave;
if (!ether_addr_equal_64bits(client_info->mac_dst, mac_bcast)) {
@@ -669,11 +692,11 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
}
if (!client_info->assigned) {
- u32 prev_tbl_head = bond_info->rx_hashtbl_head;
- bond_info->rx_hashtbl_head = hash_index;
- client_info->next = prev_tbl_head;
+ u32 prev_tbl_head = bond_info->rx_hashtbl_used_head;
+ bond_info->rx_hashtbl_used_head = hash_index;
+ client_info->used_next = prev_tbl_head;
if (prev_tbl_head != RLB_NULL_INDEX) {
- bond_info->rx_hashtbl[prev_tbl_head].prev =
+ bond_info->rx_hashtbl[prev_tbl_head].used_prev =
hash_index;
}
client_info->assigned = 1;
@@ -740,8 +763,8 @@ static void rlb_rebalance(struct bonding *bond)
_lock_rx_hashtbl_bh(bond);
ntt = 0;
- hash_index = bond_info->rx_hashtbl_head;
- for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->next) {
+ hash_index = bond_info->rx_hashtbl_used_head;
+ for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->used_next) {
client_info = &(bond_info->rx_hashtbl[hash_index]);
assigned_slave = rlb_next_rx_slave(bond);
if (assigned_slave && (client_info->slave != assigned_slave)) {
@@ -759,11 +782,113 @@ static void rlb_rebalance(struct bonding *bond)
}
/* Caller must hold rx_hashtbl lock */
+static void rlb_init_table_entry_dst(struct rlb_client_info *entry)
+{
+ entry->used_next = RLB_NULL_INDEX;
+ entry->used_prev = RLB_NULL_INDEX;
+ entry->assigned = 0;
+ entry->slave = NULL;
+ entry->tag = 0;
+}
+static void rlb_init_table_entry_src(struct rlb_client_info *entry)
+{
+ entry->src_first = RLB_NULL_INDEX;
+ entry->src_prev = RLB_NULL_INDEX;
+ entry->src_next = RLB_NULL_INDEX;
+}
+
static void rlb_init_table_entry(struct rlb_client_info *entry)
{
memset(entry, 0, sizeof(struct rlb_client_info));
- entry->next = RLB_NULL_INDEX;
- entry->prev = RLB_NULL_INDEX;
+ rlb_init_table_entry_dst(entry);
+ rlb_init_table_entry_src(entry);
+}
+
+static void rlb_delete_table_entry_dst(struct bonding *bond, u32 index)
+{
+ struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
+ u32 next_index = bond_info->rx_hashtbl[index].used_next;
+ u32 prev_index = bond_info->rx_hashtbl[index].used_prev;
+
+ if (index == bond_info->rx_hashtbl_used_head)
+ bond_info->rx_hashtbl_used_head = next_index;
+ if (prev_index != RLB_NULL_INDEX)
+ bond_info->rx_hashtbl[prev_index].used_next = next_index;
+ if (next_index != RLB_NULL_INDEX)
+ bond_info->rx_hashtbl[next_index].used_prev = prev_index;
+}
+
+/* unlink a rlb hash table entry from the src list */
+static void rlb_src_unlink(struct bonding *bond, u32 index)
+{
+ struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
+ u32 next_index = bond_info->rx_hashtbl[index].src_next;
+ u32 prev_index = bond_info->rx_hashtbl[index].src_prev;
+
+ bond_info->rx_hashtbl[index].src_next = RLB_NULL_INDEX;
+ bond_info->rx_hashtbl[index].src_prev = RLB_NULL_INDEX;
+
+ if (next_index != RLB_NULL_INDEX)
+ bond_info->rx_hashtbl[next_index].src_prev = prev_index;
+
+ if (prev_index == RLB_NULL_INDEX)
+ return;
+
+ /* is prev_index pointing to the head of this list? */
+ if (bond_info->rx_hashtbl[prev_index].src_first == index)
+ bond_info->rx_hashtbl[prev_index].src_first = next_index;
+ else
+ bond_info->rx_hashtbl[prev_index].src_next = next_index;
+
+}
+
+static void rlb_delete_table_entry(struct bonding *bond, u32 index)
+{
+ struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
+ struct rlb_client_info *entry = &(bond_info->rx_hashtbl[index]);
+
+ rlb_delete_table_entry_dst(bond, index);
+ rlb_init_table_entry_dst(entry);
+
+ rlb_src_unlink(bond, index);
+}
+
+/* add the rx_hashtbl[ip_dst_hash] entry to the list
+ * of entries with identical ip_src_hash
+ */
+static void rlb_src_link(struct bonding *bond, u32 ip_src_hash, u32 ip_dst_hash)
+{
+ struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
+ u32 next;
+
+ bond_info->rx_hashtbl[ip_dst_hash].src_prev = ip_src_hash;
+ next = bond_info->rx_hashtbl[ip_src_hash].src_first;
+ bond_info->rx_hashtbl[ip_dst_hash].src_next = next;
+ if (next != RLB_NULL_INDEX)
+ bond_info->rx_hashtbl[next].src_prev = ip_dst_hash;
+ bond_info->rx_hashtbl[ip_src_hash].src_first = ip_dst_hash;
+}
+
+/* deletes all rx_hashtbl entries with arp->ip_src if their mac_src does
+ * not match arp->mac_src */
+static void rlb_purge_src_ip(struct bonding *bond, struct arp_pkt *arp)
+{
+ struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
+ u32 ip_src_hash = _simple_hash((u8*)&(arp->ip_src), sizeof(arp->ip_src));
+ u32 index;
+
+ _lock_rx_hashtbl_bh(bond);
+
+ index = bond_info->rx_hashtbl[ip_src_hash].src_first;
+ while (index != RLB_NULL_INDEX) {
+ struct rlb_client_info *entry = &(bond_info->rx_hashtbl[index]);
+ u32 next_index = entry->src_next;
+ if (entry->ip_src == arp->ip_src &&
+ !ether_addr_equal_64bits(arp->mac_src, entry->mac_src))
+ rlb_delete_table_entry(bond, index);
+ index = next_index;
+ }
+ _unlock_rx_hashtbl_bh(bond);
}
static int rlb_initialize(struct bonding *bond)
@@ -781,7 +906,7 @@ static int rlb_initialize(struct bonding *bond)
bond_info->rx_hashtbl = new_hashtbl;
- bond_info->rx_hashtbl_head = RLB_NULL_INDEX;
+ bond_info->rx_hashtbl_used_head = RLB_NULL_INDEX;
for (i = 0; i < RLB_HASH_TABLE_SIZE; i++) {
rlb_init_table_entry(bond_info->rx_hashtbl + i);
@@ -803,7 +928,7 @@ static void rlb_deinitialize(struct bonding *bond)
kfree(bond_info->rx_hashtbl);
bond_info->rx_hashtbl = NULL;
- bond_info->rx_hashtbl_head = RLB_NULL_INDEX;
+ bond_info->rx_hashtbl_used_head = RLB_NULL_INDEX;
_unlock_rx_hashtbl_bh(bond);
}
@@ -815,25 +940,13 @@ static void rlb_clear_vlan(struct bonding *bond, unsigned short vlan_id)
_lock_rx_hashtbl_bh(bond);
- curr_index = bond_info->rx_hashtbl_head;
+ curr_index = bond_info->rx_hashtbl_used_head;
while (curr_index != RLB_NULL_INDEX) {
struct rlb_client_info *curr = &(bond_info->rx_hashtbl[curr_index]);
- u32 next_index = bond_info->rx_hashtbl[curr_index].next;
- u32 prev_index = bond_info->rx_hashtbl[curr_index].prev;
-
- if (curr->tag && (curr->vlan_id == vlan_id)) {
- if (curr_index == bond_info->rx_hashtbl_head) {
- bond_info->rx_hashtbl_head = next_index;
- }
- if (prev_index != RLB_NULL_INDEX) {
- bond_info->rx_hashtbl[prev_index].next = next_index;
- }
- if (next_index != RLB_NULL_INDEX) {
- bond_info->rx_hashtbl[next_index].prev = prev_index;
- }
+ u32 next_index = bond_info->rx_hashtbl[curr_index].used_next;
- rlb_init_table_entry(curr);
- }
+ if (curr->tag && (curr->vlan_id == vlan_id))
+ rlb_delete_table_entry(bond, curr_index);
curr_index = next_index;
}
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index 90f140a..1fbc938 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -100,9 +100,18 @@ struct tlb_client_info {
struct rlb_client_info {
__be32 ip_src; /* the server IP address */
__be32 ip_dst; /* the client IP address */
+ u8 mac_src[ETH_ALEN]; /* the server MAC address */
u8 mac_dst[ETH_ALEN]; /* the client MAC address */
- u32 next; /* The next Hash table entry index */
- u32 prev; /* The previous Hash table entry index */
+
+ /* list of used hash table entries, starting at rx_hashtbl_used_head */
+ u32 used_next;
+ u32 used_prev;
+
+ /* ip_src based hashing */
+ u32 src_next; /* next entry with same hash(ip_src) */
+ u32 src_prev; /* prev entry with same hash(ip_src) */
+ u32 src_first; /* first entry with hash(ip_src) == this entry's index */
+
u8 assigned; /* checking whether this entry is assigned */
u8 ntt; /* flag - need to transmit client info */
struct slave *slave; /* the slave assigned to this client */
@@ -131,7 +140,7 @@ struct alb_bond_info {
int rlb_enabled;
struct rlb_client_info *rx_hashtbl; /* Receive hash table */
spinlock_t rx_hashtbl_lock;
- u32 rx_hashtbl_head;
+ u32 rx_hashtbl_used_head;
u8 rx_ntt; /* flag - need to transmit
* to all rx clients
*/
diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c
index 3680aa2..a570843 100644
--- a/drivers/net/bonding/bond_debugfs.c
+++ b/drivers/net/bonding/bond_debugfs.c
@@ -31,8 +31,8 @@ static int bond_debug_rlb_hash_show(struct seq_file *m, void *v)
spin_lock_bh(&(BOND_ALB_INFO(bond).rx_hashtbl_lock));
- hash_index = bond_info->rx_hashtbl_head;
- for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->next) {
+ hash_index = bond_info->rx_hashtbl_used_head;
+ for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->used_next) {
client_info = &(bond_info->rx_hashtbl[hash_index]);
seq_printf(m, "%-15pI4 %-15pI4 %-17pM %s\n",
&client_info->ip_src,
--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ
^ permalink raw reply related
* Re: [PATCH] net: Update netdev_alloc_frag to work more efficiently with TCP and GRO
From: Alexander Duyck @ 2012-06-20 20:10 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem, jeffrey.t.kirsher
In-Reply-To: <1340217693.4604.1576.camel@edumazet-glaptop>
On 06/20/2012 11:41 AM, Eric Dumazet wrote:
> On Wed, 2012-06-20 at 10:14 -0700, Alexander Duyck wrote:
>
>> Actually I think I just realized what the difference is. I was looking
>> at things with LRO disabled. With LRO enabled our hardware RSC feature
>> kind of defeats the whole point of the GRO or TCP coalescing anyway
>> since it will stuff 16 fragments into a single packet before we even
>> hand the packet off to the stack.
> I noticed LRO was now 'off' by default on ixgbe (net-next tree), I am
> pretty sure it was 'on' some months ago ?
It should be on by default unless you are doing some routing. In that
case as soon as the interface comes up the LRO is disabled by the stack.
Thanks,
Alex
^ permalink raw reply
* Re: [patch net-next] team: do RCU update path fixups
From: Eric Dumazet @ 2012-06-20 19:14 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, jbrouer, paulmck, wfg
In-Reply-To: <1340217579-1478-1-git-send-email-jpirko@redhat.com>
On Wed, 2012-06-20 at 20:39 +0200, Jiri Pirko wrote:
> Use rcu_access_pointer and rcu_dereference_protected
> to access RCU pointer by updater.
> Use RCU_INIT_POINTER for NULL assignment of RCU pointer.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
> drivers/net/team/team_mode_activebackup.c | 8 ++++++--
> drivers/net/team/team_mode_loadbalance.c | 14 ++++++++++----
> 2 files changed, 16 insertions(+), 6 deletions(-)
Seems good to me, thanks.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH] net: Update netdev_alloc_frag to work more efficiently with TCP and GRO
From: Eric Dumazet @ 2012-06-20 18:41 UTC (permalink / raw)
To: Alexander Duyck; +Cc: netdev, davem, jeffrey.t.kirsher
In-Reply-To: <4FE20511.4000206@intel.com>
On Wed, 2012-06-20 at 10:14 -0700, Alexander Duyck wrote:
> Actually I think I just realized what the difference is. I was looking
> at things with LRO disabled. With LRO enabled our hardware RSC feature
> kind of defeats the whole point of the GRO or TCP coalescing anyway
> since it will stuff 16 fragments into a single packet before we even
> hand the packet off to the stack.
I noticed LRO was now 'off' by default on ixgbe (net-next tree), I am
pretty sure it was 'on' some months ago ?
^ permalink raw reply
* Re: [PATCH v2] ipv4: Early TCP socket demux.
From: Eric Dumazet @ 2012-06-20 18:40 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, shemminger, netdev
In-Reply-To: <1340215664.2576.12.camel@bwh-desktop.uk.solarflarecom.com>
On Wed, 2012-06-20 at 19:07 +0100, Ben Hutchings wrote:
> I don't know what you mean by 'have no use'. It's enabled anyway, and I
> would expect that it's beneficial for some smaller routers.
>
I believe vast majority of linux powered devices are more hosts,
not routers (ip_forward default value is 0 by the way)
If someone wants to tune its linux router, he probably already disables
GRO because of various issues with too big packets.
GRO adds a significant cost to forwarding path.
^ permalink raw reply
* [patch net-next] team: do RCU update path fixups
From: Jiri Pirko @ 2012-06-20 18:39 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, jbrouer, paulmck, wfg
Use rcu_access_pointer and rcu_dereference_protected
to access RCU pointer by updater.
Use RCU_INIT_POINTER for NULL assignment of RCU pointer.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/team/team_mode_activebackup.c | 8 ++++++--
drivers/net/team/team_mode_loadbalance.c | 14 ++++++++++----
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/net/team/team_mode_activebackup.c b/drivers/net/team/team_mode_activebackup.c
index 2fe02a8..253b8a5 100644
--- a/drivers/net/team/team_mode_activebackup.c
+++ b/drivers/net/team/team_mode_activebackup.c
@@ -61,8 +61,12 @@ static void ab_port_leave(struct team *team, struct team_port *port)
static int ab_active_port_get(struct team *team, struct team_gsetter_ctx *ctx)
{
- if (ab_priv(team)->active_port)
- ctx->data.u32_val = ab_priv(team)->active_port->dev->ifindex;
+ struct team_port *active_port;
+
+ active_port = rcu_dereference_protected(ab_priv(team)->active_port,
+ lockdep_is_held(&team->lock));
+ if (active_port)
+ ctx->data.u32_val = active_port->dev->ifindex;
else
ctx->data.u32_val = 0;
return 0;
diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
index 45cc095..c92fa02 100644
--- a/drivers/net/team/team_mode_loadbalance.c
+++ b/drivers/net/team/team_mode_loadbalance.c
@@ -96,8 +96,8 @@ static void lb_tx_hash_to_port_mapping_null_port(struct team *team,
struct lb_port_mapping *pm;
pm = &lb_priv->ex->tx_hash_to_port_mapping[i];
- if (pm->port == port) {
- rcu_assign_pointer(pm->port, NULL);
+ if (rcu_access_pointer(pm->port) == port) {
+ RCU_INIT_POINTER(pm->port, NULL);
team_option_inst_set_change(pm->opt_inst_info);
changed = true;
}
@@ -274,6 +274,7 @@ static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx)
{
struct lb_priv *lb_priv = get_lb_priv(team);
struct sk_filter *fp = NULL;
+ struct sk_filter *orig_fp;
struct sock_fprog *fprog = NULL;
int err;
@@ -292,7 +293,9 @@ static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx)
if (lb_priv->ex->orig_fprog) {
/* Clear old filter data */
__fprog_destroy(lb_priv->ex->orig_fprog);
- sk_unattached_filter_destroy(lb_priv->fp);
+ orig_fp = rcu_dereference_protected(lb_priv->fp,
+ lockdep_is_held(&team->lock));
+ sk_unattached_filter_destroy(orig_fp);
}
rcu_assign_pointer(lb_priv->fp, fp);
@@ -303,9 +306,12 @@ static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx)
static int lb_tx_method_get(struct team *team, struct team_gsetter_ctx *ctx)
{
struct lb_priv *lb_priv = get_lb_priv(team);
+ lb_select_tx_port_func_t *func;
char *name;
- name = lb_select_tx_port_get_name(lb_priv->select_tx_port_func);
+ func = rcu_dereference_protected(lb_priv->select_tx_port_func,
+ lockdep_is_held(&team->lock));
+ name = lb_select_tx_port_get_name(func);
BUG_ON(!name);
ctx->data.str_val = name;
return 0;
--
1.7.10.4
^ permalink raw reply related
* Re: next-20120620 build error in netfilter
From: valdis.kletnieks @ 2012-06-20 18:37 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netdev, linux-kernel
In-Reply-To: <20120620172801.GA9385@1984>
[-- Attachment #1: Type: text/plain, Size: 617 bytes --]
On Wed, 20 Jun 2012 19:28:01 +0200, Pablo Neira Ayuso said:
> On Wed, Jun 20, 2012 at 01:14:33PM -0400, Valdis Kletnieks wrote:
> > Today's linux-next fails to build with CONFIG_NF_NAT_NEEDED=y and CONFIG_NF_NAT=m
> >
> > LD init/built-in.o
> > net/built-in.o:(.data+0x4408): undefined reference to `nf_nat_tcp_seq_adjust'
> > make: *** [vmlinux] Error 1
>
> I guess you have NF_CT_NETLINK=y, right?
Yes.
Temporary workaround was setting NF_NAT=n (I don't use NAT, I just usually
build all the netfilter modules as =m even if I don't use them, just to provide
build coverage and trip over stuff like this).
[-- Attachment #2: Type: application/pgp-signature, Size: 865 bytes --]
^ permalink raw reply
* Re: [patch net-next 1/2] team: use rcu_access_pointer to access RCU pointer by writer
From: Jiri Pirko @ 2012-06-20 18:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem, jbrouer, paulmck, wfg
In-Reply-To: <1340208104.4604.1247.camel@edumazet-glaptop>
Wed, Jun 20, 2012 at 06:01:44PM CEST, eric.dumazet@gmail.com wrote:
>On Wed, 2012-06-20 at 17:32 +0200, Jiri Pirko wrote:
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>> ---
>> drivers/net/team/team_mode_activebackup.c | 7 +++++--
>> drivers/net/team/team_mode_loadbalance.c | 8 +++++---
>> 2 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/team/team_mode_activebackup.c b/drivers/net/team/team_mode_activebackup.c
>> index 2fe02a8..c9e7621 100644
>> --- a/drivers/net/team/team_mode_activebackup.c
>> +++ b/drivers/net/team/team_mode_activebackup.c
>> @@ -61,8 +61,11 @@ static void ab_port_leave(struct team *team, struct team_port *port)
>>
>> static int ab_active_port_get(struct team *team, struct team_gsetter_ctx *ctx)
>> {
>> - if (ab_priv(team)->active_port)
>> - ctx->data.u32_val = ab_priv(team)->active_port->dev->ifindex;
>> + struct team_port *active_port;
>> +
>> + active_port = rcu_access_pointer(ab_priv(team)->active_port);
>
>This is not the correct fix.
>
>You cant safely dereference active_port if you got it from
>rcu_access_pointer()
>
>You should use rcu_dereference() of rcu_dereference_protected() or
>rcu_dereference_bh() or similar variant, depending on the context.
Okay, reworking this using rcu_dereference_protected() since this is
update path.
>
>> + if (active_port)
>> + ctx->data.u32_val = active_port->dev->ifindex;
>> else
>> ctx->data.u32_val = 0;
>> return 0;
>> diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
>> index 45cc095..b4475a5 100644
>> --- a/drivers/net/team/team_mode_loadbalance.c
>> +++ b/drivers/net/team/team_mode_loadbalance.c
>> @@ -96,7 +96,7 @@ static void lb_tx_hash_to_port_mapping_null_port(struct team *team,
>> struct lb_port_mapping *pm;
>>
>> pm = &lb_priv->ex->tx_hash_to_port_mapping[i];
>> - if (pm->port == port) {
>> + if (rcu_access_pointer(pm->port) == port) {
>
>This one is OK
>
>> rcu_assign_pointer(pm->port, NULL);
>
>I dont understand why you submit two patches...
Squashing into one now.
>
>> team_option_inst_set_change(pm->opt_inst_info);
>> changed = true;
>> @@ -292,7 +292,7 @@ static int lb_bpf_func_set(struct team *team, struct team_gsetter_ctx *ctx)
>> if (lb_priv->ex->orig_fprog) {
>> /* Clear old filter data */
>> __fprog_destroy(lb_priv->ex->orig_fprog);
>> - sk_unattached_filter_destroy(lb_priv->fp);
>> + sk_unattached_filter_destroy(rcu_access_pointer(lb_priv->fp));
>> }
>
>
^ permalink raw reply
* Re: [PATCH v2] ipv4: Early TCP socket demux.
From: Ben Hutchings @ 2012-06-20 18:16 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20120619.190202.2133868658770269780.davem@davemloft.net>
On Tue, 2012-06-19 at 19:02 -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 19 Jun 2012 18:05:27 -0700 (PDT)
>
> > From: Ben Hutchings <bhutchings@solarflare.com>
> > Date: Wed, 20 Jun 2012 02:03:26 +0100
> >
> >> On Tue, 2012-06-19 at 17:54 -0700, David Miller wrote:
> >>> The hash is perfect, what's the big deal?
> >>
> >> It obscures what we're really doing and relying on.
> >
> > If it matters to you, patches are always welcome :-)
>
> Nevermind, I just committed the following to net-next:
[...]
Thanks very much, David.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v2] ipv4: Early TCP socket demux.
From: Ben Hutchings @ 2012-06-20 18:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, shemminger, netdev
In-Reply-To: <1340171940.4604.799.camel@edumazet-glaptop>
On Wed, 2012-06-20 at 07:59 +0200, Eric Dumazet wrote:
> On Tue, 2012-06-19 at 21:46 -0700, David Miller wrote:
>
> > These numbers can be decreased further, because since we're already
> > looking at the TCP header we can pre-cook the TCP control block in the
> > SKB and skip much of the stuff that tcp_v4_rcv() does since we've done
> > it already in the early demux code.
>
> It could be done at GRO level and remove one another demux.
>
> As routers probably have no use of GRO, no need of additional knob.
I don't know what you mean by 'have no use'. It's enabled anyway, and I
would expect that it's beneficial for some smaller routers.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* RE: [PATCH net-next 2/6] bnx2x: link cleanup
From: Yuval Mintz @ 2012-06-20 17:50 UTC (permalink / raw)
To: Joe Perches
Cc: davem@davemloft.net, netdev@vger.kernel.org, Eilon Greenstein,
Yaniv Rosner
In-Reply-To: <1340204019.29885.9.camel@joe2Laptop>
> > 3. Change msleep(1) --> usleep_range(1000, 1000)
>
> I believe replacing msleep(small) with
> usleep_range(small * 1000, small * 1000) is
> not generally a good idea.
>
> Please give usleep_range an actual range to
> work with and not a repeated single value.
>
> Please think a little more about what a
> good upper range for the maximum time to
> sleep should be.
>
> usleep_range(small * 1000, small * 2000)
> or something similar maybe.
>
Sounds good. I'll change it and re-send the patch series.
Thanks,
Yuval Mintz
^ permalink raw reply
* Re: [PATCH 12/12] Avoid dereferencing bd_disk during swap_entry_free for network storage
From: Rik van Riel @ 2012-06-20 17:47 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, Linux-MM, Linux-Netdev, Linux-NFS, LKML,
David Miller, Trond Myklebust, Neil Brown, Christoph Hellwig,
Peter Zijlstra, Mike Christie, Eric B Munson
In-Reply-To: <1340185081-22525-13-git-send-email-mgorman@suse.de>
On 06/20/2012 05:38 AM, Mel Gorman wrote:
> Commit [b3a27d: swap: Add swap slot free callback to
> block_device_operations] dereferences p->bdev->bd_disk but this is a
> NULL dereference if using swap-over-NFS. This patch checks SWP_BLKDEV
> on the swap_info_struct before dereferencing.
>
> With reference to this callback, Christoph Hellwig stated "Please
> just remove the callback entirely. It has no user outside the staging
> tree and was added clearly against the rules for that staging tree".
> This would also be my preference but there was not an obvious way of
> keeping zram in staging/ happy.
>
> Signed-off-by: Xiaotian Feng<dfeng@redhat.com>
> Signed-off-by: Mel Gorman<mgorman@suse.de>
Acked-by: Rik van Riel<riel@redhat.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 11/12] nfs: Prevent page allocator recursions with swap over NFS.
From: Rik van Riel @ 2012-06-20 17:42 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, Linux-MM, Linux-Netdev, Linux-NFS, LKML,
David Miller, Trond Myklebust, Neil Brown, Christoph Hellwig,
Peter Zijlstra, Mike Christie, Eric B Munson
In-Reply-To: <1340185081-22525-12-git-send-email-mgorman@suse.de>
On 06/20/2012 05:38 AM, Mel Gorman wrote:
> GFP_NOFS is _more_ permissive than GFP_NOIO in that it will initiate
> IO, just not of any filesystem data.
>
> The problem is that previously NOFS was correct because that avoids
> recursion into the NFS code. With swap-over-NFS, it is no longer
> correct as swap IO can lead to this recursion.
>
> Signed-off-by: Peter Zijlstra<a.p.zijlstra@chello.nl>
> Signed-off-by: Mel Gorman<mgorman@suse.de>
Acked-by: Rik van Riel<riel@redhat.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 10/12] nfs: enable swap on NFS
From: Rik van Riel @ 2012-06-20 17:38 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, Linux-MM, Linux-Netdev, Linux-NFS, LKML,
David Miller, Trond Myklebust, Neil Brown, Christoph Hellwig,
Peter Zijlstra, Mike Christie, Eric B Munson
In-Reply-To: <1340185081-22525-11-git-send-email-mgorman@suse.de>
On 06/20/2012 05:37 AM, Mel Gorman wrote:
> Implement the new swapfile a_ops for NFS and hook up ->direct_IO. This
> will set the NFS socket to SOCK_MEMALLOC and run socket reconnect
> under PF_MEMALLOC as well as reset SOCK_MEMALLOC before engaging the
> protocol ->connect() method.
>
> PF_MEMALLOC should allow the allocation of struct socket and related
> objects and the early (re)setting of SOCK_MEMALLOC should allow us
> to receive the packets required for the TCP connection buildup.
>
> [dfeng@redhat.com: Fix handling of multiple swap files]
> [a.p.zijlstra@chello.nl: Original patch]
> Signed-off-by: Mel Gorman<mgorman@suse.de>
Acked-by: Rik van Riel<riel@redhat.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: next-20120620 build error in netfilter
From: Pablo Neira Ayuso @ 2012-06-20 17:28 UTC (permalink / raw)
To: Valdis Kletnieks; +Cc: netdev, linux-kernel
In-Reply-To: <32596.1340212473@turing-police.cc.vt.edu>
On Wed, Jun 20, 2012 at 01:14:33PM -0400, Valdis Kletnieks wrote:
> Today's linux-next fails to build with CONFIG_NF_NAT_NEEDED=y and CONFIG_NF_NAT=m
>
> LD init/built-in.o
> net/built-in.o:(.data+0x4408): undefined reference to `nf_nat_tcp_seq_adjust'
> make: *** [vmlinux] Error 1
I guess you have NF_CT_NETLINK=y, right?
> Breakage introduced with this commit:
>
> commit 8c88f87cb27ad09086940bdd3e6955e5325ec89a
> Author: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Thu Jun 7 13:31:25 2012 +0200
>
> netfilter: nfnetlink_queue: add NAT TCP sequence adjustment if packet mangled
>
^ permalink raw reply
* Re: [net-next.git 4/4 (v7)] phy: add the EEE support and the way to access to the MMD registers.
From: Ben Hutchings @ 2012-06-20 17:22 UTC (permalink / raw)
To: Giuseppe CAVALLARO; +Cc: netdev, eric.dumazet, rayagond, davem, yuvalmin
In-Reply-To: <1340172774-27443-5-git-send-email-peppe.cavallaro@st.com>
On Wed, 2012-06-20 at 08:12 +0200, Giuseppe CAVALLARO wrote:
[...]
> +int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable)
> +{
> + int ret = -EPROTONOSUPPORT;
> +
> + /* According to 802.3az,the EEE is supported only in full duplex-mode.
> + * Also EEE feature is active when core is operating with MII, GMII
> + * or RGMII.
> + */
> + if ((phydev->duplex == DUPLEX_FULL) &&
> + ((phydev->interface == PHY_INTERFACE_MODE_MII) ||
> + (phydev->interface == PHY_INTERFACE_MODE_GMII) ||
> + (phydev->interface == PHY_INTERFACE_MODE_RGMII))) {
> + u16 eee_lp, eee_cap, eee_adv;
> + u32 lp, cap, adv;
> + int idx;
> +
> + /* Read phy status to properly get the right settings */
> + ret = phy_read_status(phydev);
> + if (ret)
> + return ret;
> +
> + /* First check if the EEE ability is supported */
> + eee_cap = phy_read_mmd_indirect(phydev->bus, MDIO_PCS_EEE_ABLE,
> + MDIO_MMD_PCS, phydev->addr);
> + if (eee_cap < 0)
> + return eee_cap;
> +
> + cap = phy_eee_to_supported(eee_cap);
> + if (!cap)
> + goto eee_exit;
But this now means returning 0 since you added the call to
phy_read_status() above. Maybe you should just return -EPROTONOSUPPORT
directly instead of relying on the assumed value of ret?
> + /* Check which link settings negotiated and verify it in
> + * the EEE advertising registers.
> + */
> + eee_lp = phy_read_mmd_indirect(phydev->bus, MDIO_AN_EEE_LPABLE,
> + MDIO_MMD_AN, phydev->addr);
> + if (eee_lp < 0)
> + return eee_lp;
> +
> + eee_adv = phy_read_mmd_indirect(phydev->bus, MDIO_AN_EEE_ADV,
> + MDIO_MMD_AN, phydev->addr);
> + if (eee_adv < 0)
> + return eee_adv;
> +
> + adv = phy_eee_to_adv(eee_adv);
> + lp = phy_eee_to_adv(eee_lp);
> + idx = phy_find_setting(phydev->speed, phydev->duplex);
> + if ((lp & adv & settings[idx].setting))
> + goto eee_exit;
[...]
Same problem here.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Q: NET/JME: pci_get_drvdata pointer return check at jme_remove
From: devendra.aaru @ 2012-06-20 17:20 UTC (permalink / raw)
To: netdev
Hi,
looking at the jme_init_one error path, the context of the driver data
is set to null.
If the driver unloads , the unload _remove_one, will be called and
deferencing the pointer, leading to a oops.
so we need to have a check at before doing the netdev_priv at remove_one.
Please correct me if my understanding is wrong....
Thanks,
Devendra.
^ permalink raw reply
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