Netdev List
 help / color / mirror / Atom feed
* [RFC PATCH 3/6] l2tp: drop session refs in l2tp_tunnel_closeall
From: Tom Parkin @ 2013-02-15 10:25 UTC (permalink / raw)
  To: netdev; +Cc: jchapman, celston, Tom Parkin
In-Reply-To: <1360923919-7203-1-git-send-email-tparkin@katalix.com>

l2tp_tunnel_closeall is intended to close and free all sessions running in a
given tunnel.  It lacked a session dereference, however, meaning these
sessions would all leak rather than being properly freed.

Add a session deref call in l2tp_tunnel_closeall to prevent this leak.

---
 net/l2tp/l2tp_core.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 3842f67..7f3ab65 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1328,6 +1328,8 @@ again:
 			if (session->deref != NULL)
 				(*session->deref)(session);
 
+			l2tp_session_dec_refcount(session);
+
 			write_lock_bh(&tunnel->hlist_lock);
 
 			/* Now restart from the beginning of this hash
-- 
1.7.9.5

^ permalink raw reply related

* [RFC PATCH 4/6] l2tp: export l2tp_tunnel_closeall
From: Tom Parkin @ 2013-02-15 10:25 UTC (permalink / raw)
  To: netdev; +Cc: jchapman, celston, Tom Parkin
In-Reply-To: <1360923919-7203-1-git-send-email-tparkin@katalix.com>

l2tp_core internally uses l2tp_tunnel_closeall to close all sessions in a
tunnel when a UDP-encapsualtion socket is destroyed.  We need to do something
similar for IP-encapsualtion sockets.

Export l2tp_tunnel_closeall as a GPL symbol to enable l2tp_ip and l2tp_ip6 to
call it from their .destroy handlers.

---
 net/l2tp/l2tp_core.c |    4 ++--
 net/l2tp/l2tp_core.h |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 7f3ab65..26b5f00 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -114,7 +114,6 @@ struct l2tp_net {
 
 static void l2tp_session_set_header_len(struct l2tp_session *session, int version);
 static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel);
-static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel);
 
 static inline struct l2tp_net *l2tp_pernet(struct net *net)
 {
@@ -1279,7 +1278,7 @@ end:
 
 /* When the tunnel is closed, all the attached sessions need to go too.
  */
-static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
+void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
 {
 	int hash;
 	struct hlist_node *walk;
@@ -1342,6 +1341,7 @@ again:
 	}
 	write_unlock_bh(&tunnel->hlist_lock);
 }
+EXPORT_SYMBOL_GPL(l2tp_tunnel_closeall);
 
 /* Tunnel socket destroy hook for UDP encapsulation */
 static void l2tp_udp_encap_destroy(struct sock *sk)
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index a6d9c5d..d1b37c1 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -239,6 +239,7 @@ extern struct l2tp_tunnel *l2tp_tunnel_find(struct net *net, u32 tunnel_id);
 extern struct l2tp_tunnel *l2tp_tunnel_find_nth(struct net *net, int nth);
 
 extern int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32 peer_tunnel_id, struct l2tp_tunnel_cfg *cfg, struct l2tp_tunnel **tunnelp);
+extern void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel);
 extern int l2tp_tunnel_delete(struct l2tp_tunnel *tunnel);
 extern struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunnel, u32 session_id, u32 peer_session_id, struct l2tp_session_cfg *cfg);
 extern int l2tp_session_delete(struct l2tp_session *session);
-- 
1.7.9.5

^ permalink raw reply related

* [RFC PATCH 5/6] l2tp: close sessions in ip socket destroy callback
From: Tom Parkin @ 2013-02-15 10:25 UTC (permalink / raw)
  To: netdev; +Cc: jchapman, celston, Tom Parkin
In-Reply-To: <1360923919-7203-1-git-send-email-tparkin@katalix.com>

l2tp_core hooks UDP's .destroy handler to gain advance warning of a tunnel
socket being closed from userspace.  We need to do the same thing for
IP-encapsualtion sockets.

---
 net/l2tp/l2tp_ip.c  |    6 ++++++
 net/l2tp/l2tp_ip6.c |    7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index f7ac8f4..338262d 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -229,10 +229,16 @@ static void l2tp_ip_close(struct sock *sk, long timeout)
 static void l2tp_ip_destroy_sock(struct sock *sk)
 {
 	struct sk_buff *skb;
+	struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
 
 	while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL)
 		kfree_skb(skb);
 
+	if (tunnel) {
+		l2tp_tunnel_closeall(tunnel);
+		sock_put(sk);
+	}
+
 	sk_refcnt_debug_dec(sk);
 }
 
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 8ee4a86..b745a40 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -242,10 +242,17 @@ static void l2tp_ip6_close(struct sock *sk, long timeout)
 
 static void l2tp_ip6_destroy_sock(struct sock *sk)
 {
+	struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
+
 	lock_sock(sk);
 	ip6_flush_pending_frames(sk);
 	release_sock(sk);
 
+	if (tunnel) {
+		l2tp_tunnel_closeall(tunnel);
+		sock_put(sk);
+	}
+
 	inet6_destroy_sock(sk);
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [RFC PATCH 2/6] l2tp: add udp encap socket destroy handler
From: Tom Parkin @ 2013-02-15 10:25 UTC (permalink / raw)
  To: netdev; +Cc: jchapman, celston, Tom Parkin
In-Reply-To: <1360923919-7203-1-git-send-email-tparkin@katalix.com>

L2TP sessions hold a reference to the tunnel socket to prevent it going away
while sessions are still active.  However, since tunnel destruction is handled
by the sock sk_destruct callback there is a catch-22: a tunnel with sessions
cannot be deleted since each session holds a reference to the tunnel socket.
If userspace closes a managed tunnel socket, or dies, the tunnel will persist
and it will be neccessary to individually delete the sessions using netlink
commands.  This is ugly.

To prevent this occuring, this patch leverages the udp encapsulation socket
destroy callback to gain early notification when the tunnel socket is closed.
This allows us to safely close the sessions running in the tunnel, dropping
the tunnel socket references in the process.  The tunnel socket is then
destroyed as normal, and the tunnel resources deallocated in sk_destruct.

---
 net/l2tp/l2tp_core.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 5edf1e2..3842f67 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1249,6 +1249,7 @@ static void l2tp_tunnel_destruct(struct sock *sk)
 		/* No longer an encapsulation socket. See net/ipv4/udp.c */
 		(udp_sk(sk))->encap_type = 0;
 		(udp_sk(sk))->encap_rcv = NULL;
+		(udp_sk(sk))->encap_destroy = NULL;
 		break;
 	case L2TP_ENCAPTYPE_IP:
 		break;
@@ -1340,6 +1341,16 @@ again:
 	write_unlock_bh(&tunnel->hlist_lock);
 }
 
+/* Tunnel socket destroy hook for UDP encapsulation */
+static void l2tp_udp_encap_destroy(struct sock *sk)
+{
+	struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
+	if (tunnel) {
+		l2tp_tunnel_closeall(tunnel);
+		sock_put(sk);
+	}
+}
+
 /* Really kill the tunnel.
  * Come here only when all sessions have been cleared from the tunnel.
  */
@@ -1635,6 +1646,7 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
 		/* Mark socket as an encapsulation socket. See net/ipv4/udp.c */
 		udp_sk(sk)->encap_type = UDP_ENCAP_L2TPINUDP;
 		udp_sk(sk)->encap_rcv = l2tp_udp_encap_recv;
+		udp_sk(sk)->encap_destroy = l2tp_udp_encap_destroy;
 #if IS_ENABLED(CONFIG_IPV6)
 		if (sk->sk_family == PF_INET6)
 			udpv6_encap_enable();
-- 
1.7.9.5

^ permalink raw reply related

* [RFC PATCH 1/6] udp: add encap_destroy callback
From: Tom Parkin @ 2013-02-15 10:25 UTC (permalink / raw)
  To: netdev; +Cc: jchapman, celston, Tom Parkin
In-Reply-To: <1360923919-7203-1-git-send-email-tparkin@katalix.com>

Users of udp encapsulation currently have an encap_rcv callback which they can
use to hook into the udp receive path.

In situations where a encapsulation user allocates resources associated with a
udp encap socket, it may be convenient to be able to also hook the proto
.destroy operation.  For example, if an encap user holds a reference to the
udp socket, the destroy hook might be used to relinquish this reference.

This patch adds a socket destroy hook into udp, which is set and enabled
in the same way as the existing encap_rcv hook.

---
 include/linux/udp.h |    1 +
 net/ipv4/udp.c      |    7 +++++++
 net/ipv6/udp.c      |    8 ++++++++
 3 files changed, 16 insertions(+)

diff --git a/include/linux/udp.h b/include/linux/udp.h
index 9d81de1..42278bb 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -68,6 +68,7 @@ struct udp_sock {
 	 * For encapsulation sockets.
 	 */
 	int (*encap_rcv)(struct sock *sk, struct sk_buff *skb);
+	void (*encap_destroy)(struct sock *sk);
 };
 
 static inline struct udp_sock *udp_sk(const struct sock *sk)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 6791aac..48e2366 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1762,9 +1762,16 @@ int udp_rcv(struct sk_buff *skb)
 
 void udp_destroy_sock(struct sock *sk)
 {
+	struct udp_sock *up = udp_sk(sk);
 	bool slow = lock_sock_fast(sk);
 	udp_flush_pending_frames(sk);
 	unlock_sock_fast(sk, slow);
+	if (static_key_false(&udp_encap_needed) && up->encap_type) {
+		void (*encap_destroy)(struct sock *sk);
+		encap_destroy = ACCESS_ONCE(up->encap_destroy);
+		if (encap_destroy)
+			encap_destroy(sk);
+	}
 }
 
 /*
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 599e1ba6..d8e5e85 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1285,10 +1285,18 @@ do_confirm:
 
 void udpv6_destroy_sock(struct sock *sk)
 {
+	struct udp_sock *up = udp_sk(sk);
 	lock_sock(sk);
 	udp_v6_flush_pending_frames(sk);
 	release_sock(sk);
 
+	if (static_key_false(&udpv6_encap_needed) && up->encap_type) {
+		void (*encap_destroy)(struct sock *sk);
+		encap_destroy = ACCESS_ONCE(up->encap_destroy);
+		if (encap_destroy)
+			encap_destroy(sk);
+	}
+
 	inet6_destroy_sock(sk);
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [RFC PATCH 0/6] udp: add encapsulation socket destroy hook
From: Tom Parkin @ 2013-02-15 10:25 UTC (permalink / raw)
  To: netdev; +Cc: jchapman, celston, Tom Parkin

While working on an issue in l2tp relating to tunnel cleanup I discovered a
problem which I think may require a new hook in udp core to resolve.  The l2tp
code uses encapsulated udp sockets, and it needs to know when a socket is
being closed in order to clean up.  Since sessions hold a reference to the
tunnel socket, hooking sk_destruct isn't sufficient: any tunnel with sessions
running in it will be pinned open by the sessions.

To resolve this, the first patch in this series adds a .destroy hook to udp, 
while the following patches show how I'd use this feature in l2tp.  Essentially 
the .destroy callback is used to close the sessions in the tunnel, thereby 
dropping the socket references.

Does this seem like a reasonable approach?  I'm reluctant to modify udp, but
I've not managed to discover a safe alternative...

The rest of this email goes into some more detail about the problem I'm trying
to solve.

A bit of background is probably useful for context.

The kernel l2tp code supports two flavours of tunnel socket: managed and
unmanaged.  The managed tunnel socket is created by a userspace daemon which
handles the L2TP control protocol, while the kernel handles L2TP data
encapsulation.  The unmanaged tunnel socket is created by the kernel
and just provides L2TP data encapsulation with no control protocol.

These two flavours of tunnel socket make for a fairly complex set of tunnel
lifetime requirements:

A managed tunnel may be deleted by:

 * userspace using a netlink "delete" command
 * userspace calling close(2) on the tunnel socket
 * userspace dying and the tunnel socket being released as a part of
   the normal tidyup after a process terminates

An unmanaged tunnel may be deleted by:

 * userspace using a netlink "delete" command

In order to satisfy all these deletion mechanisms, l2tp uses sk_destruct to
free tunnel resources when the tunnel socket is being destroyed.  That way we
can catch userspace closing the socket as well as explicit deletion using
netlink.

The problem comes when sessions are added to the tunnel.  Each session holds a
reference to the tunnel socket, in order to prevent the tunnel socket
unexpectedly going away while the session is still active and passing data.
But of course this also keeps the tunnel socket alive even if userspace closes
it!

This is where the protocol .destroy hook comes in.  Because .destroy is called
by sk_common_release before sock_put, the l2tp code is able to use the hook to
close the sessions in the tunnel.  Now that all the session references to the
socket have been dropped, when sk_common_release makes its final sock_put
call, the socket is actually freed and sk_destruct is called.

Tom Parkin (6):
  udp: add encap_destroy callback
  l2tp: add udp encap socket destroy handler
  l2tp: drop session refs in l2tp_tunnel_closeall
  l2tp: export l2tp_tunnel_closeall
  l2tp: close sessions in ip socket destroy callback
  l2tp: close sessions before initiating tunnel delete

 include/linux/udp.h  |    1 +
 net/ipv4/udp.c       |    7 +++++++
 net/ipv6/udp.c       |    8 ++++++++
 net/l2tp/l2tp_core.c |   19 +++++++++++++++++--
 net/l2tp/l2tp_core.h |    1 +
 net/l2tp/l2tp_ip.c   |    6 ++++++
 net/l2tp/l2tp_ip6.c  |    7 +++++++
 7 files changed, 47 insertions(+), 2 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [net-next 09/10] igb: Refractoring function pointers in igb_get_invariants function
From: Jeff Kirsher @ 2013-02-15 10:04 UTC (permalink / raw)
  To: davem; +Cc: Akeem G. Abodunrin, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1360922649-3388-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>

This patch simplifies igb_get_invariants function by moving all implemented
function pointers in this function to individual separate functions,
based on their functionalities, this would make debugging much easier.

Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_82575.c | 262 ++-------------------------
 1 file changed, 11 insertions(+), 251 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c
index 8604013..84e7e09 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -395,13 +395,9 @@ static s32 igb_init_mac_params_82575(struct e1000_hw *hw)
 
 static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 {
-	struct e1000_phy_info *phy = &hw->phy;
-	struct e1000_nvm_info *nvm = &hw->nvm;
 	struct e1000_mac_info *mac = &hw->mac;
 	struct e1000_dev_spec_82575 * dev_spec = &hw->dev_spec._82575;
-	u32 eecd;
 	s32 ret_val;
-	u16 size;
 	u32 ctrl_ext = 0;
 
 	switch (hw->device_id) {
@@ -462,7 +458,7 @@ static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 	 * SerDes mode on the 82575. There can be an external PHY attached
 	 * on the SGMII interface. For this, we'll set sgmii_active to true.
 	 */
-	phy->media_type = e1000_media_type_copper;
+	hw->phy.media_type = e1000_media_type_copper;
 	dev_spec->sgmii_active = false;
 
 	ctrl_ext = rd32(E1000_CTRL_EXT);
@@ -478,154 +474,15 @@ static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 		break;
 	}
 
-	/* Set mta register count */
-	mac->mta_reg_count = 128;
-	/* Set rar entry count */
-	switch (mac->type) {
-	case e1000_82576:
-		mac->rar_entry_count = E1000_RAR_ENTRIES_82576;
-		break;
-	case e1000_82580:
-		mac->rar_entry_count = E1000_RAR_ENTRIES_82580;
-		break;
-	case e1000_i350:
-		mac->rar_entry_count = E1000_RAR_ENTRIES_I350;
-		break;
-	default:
-		mac->rar_entry_count = E1000_RAR_ENTRIES_82575;
-		break;
-	}
-	/* reset */
-	if (mac->type >= e1000_82580)
-		mac->ops.reset_hw = igb_reset_hw_82580;
-	else
-		mac->ops.reset_hw = igb_reset_hw_82575;
-
-	if (mac->type >= e1000_i210) {
-		mac->ops.acquire_swfw_sync = igb_acquire_swfw_sync_i210;
-		mac->ops.release_swfw_sync = igb_release_swfw_sync_i210;
-	} else {
-		mac->ops.acquire_swfw_sync = igb_acquire_swfw_sync_82575;
-		mac->ops.release_swfw_sync = igb_release_swfw_sync_82575;
-	}
-
-	/* Set if part includes ASF firmware */
-	mac->asf_firmware_present = true;
-	/* Set if manageability features are enabled. */
-	mac->arc_subsystem_valid =
-		(rd32(E1000_FWSM) & E1000_FWSM_MODE_MASK)
-			? true : false;
-	/* enable EEE on i350 parts and later parts */
-	if (mac->type >= e1000_i350)
-		dev_spec->eee_disable = false;
-	else
-		dev_spec->eee_disable = true;
-	/* physical interface link setup */
-	mac->ops.setup_physical_interface =
-		(hw->phy.media_type == e1000_media_type_copper)
-			? igb_setup_copper_link_82575
-			: igb_setup_serdes_link_82575;
+	/* mac initialization and operations */
+	ret_val = igb_init_mac_params_82575(hw);
+	if (ret_val)
+		goto out;
 
 	/* NVM initialization */
-	eecd = rd32(E1000_EECD);
-	size = (u16)((eecd & E1000_EECD_SIZE_EX_MASK) >>
-		     E1000_EECD_SIZE_EX_SHIFT);
-
-	/*
-	 * Added to a constant, "size" becomes the left-shift value
-	 * for setting word_size.
-	 */
-	size += NVM_WORD_SIZE_BASE_SHIFT;
-
-	/*
-	 * Check for invalid size
-	 */
-	if ((hw->mac.type == e1000_82576) && (size > 15)) {
-		pr_notice("The NVM size is not valid, defaulting to 32K\n");
-		size = 15;
-	}
-
-	nvm->word_size = 1 << size;
-	if (hw->mac.type < e1000_i210) {
-		nvm->opcode_bits        = 8;
-		nvm->delay_usec         = 1;
-		switch (nvm->override) {
-		case e1000_nvm_override_spi_large:
-			nvm->page_size    = 32;
-			nvm->address_bits = 16;
-			break;
-		case e1000_nvm_override_spi_small:
-			nvm->page_size    = 8;
-			nvm->address_bits = 8;
-			break;
-		default:
-			nvm->page_size    = eecd
-				& E1000_EECD_ADDR_BITS ? 32 : 8;
-			nvm->address_bits = eecd
-				& E1000_EECD_ADDR_BITS ? 16 : 8;
-			break;
-		}
-		if (nvm->word_size == (1 << 15))
-			nvm->page_size = 128;
-
-		nvm->type = e1000_nvm_eeprom_spi;
-	} else
-		nvm->type = e1000_nvm_flash_hw;
-
-	/* NVM Function Pointers */
-	switch (hw->mac.type) {
-	case e1000_82580:
-		nvm->ops.validate = igb_validate_nvm_checksum_82580;
-		nvm->ops.update = igb_update_nvm_checksum_82580;
-		nvm->ops.acquire = igb_acquire_nvm_82575;
-		nvm->ops.release = igb_release_nvm_82575;
-		if (nvm->word_size < (1 << 15))
-			nvm->ops.read = igb_read_nvm_eerd;
-		else
-			nvm->ops.read = igb_read_nvm_spi;
-		nvm->ops.write = igb_write_nvm_spi;
-		break;
-	case e1000_i350:
-		nvm->ops.validate = igb_validate_nvm_checksum_i350;
-		nvm->ops.update = igb_update_nvm_checksum_i350;
-		nvm->ops.acquire = igb_acquire_nvm_82575;
-		nvm->ops.release = igb_release_nvm_82575;
-		if (nvm->word_size < (1 << 15))
-			nvm->ops.read = igb_read_nvm_eerd;
-		else
-			nvm->ops.read = igb_read_nvm_spi;
-		nvm->ops.write = igb_write_nvm_spi;
-		break;
-	case e1000_i210:
-		nvm->ops.validate = igb_validate_nvm_checksum_i210;
-		nvm->ops.update   = igb_update_nvm_checksum_i210;
-		nvm->ops.acquire = igb_acquire_nvm_i210;
-		nvm->ops.release = igb_release_nvm_i210;
-		nvm->ops.read    = igb_read_nvm_srrd_i210;
-		nvm->ops.write   = igb_write_nvm_srwr_i210;
-		nvm->ops.valid_led_default = igb_valid_led_default_i210;
-		break;
-	case e1000_i211:
-		nvm->ops.acquire  = igb_acquire_nvm_i210;
-		nvm->ops.release  = igb_release_nvm_i210;
-		nvm->ops.read     = igb_read_nvm_i211;
-		nvm->ops.valid_led_default = igb_valid_led_default_i210;
-		nvm->ops.validate = NULL;
-		nvm->ops.update   = NULL;
-		nvm->ops.write    = NULL;
-		break;
-	default:
-		nvm->ops.validate = igb_validate_nvm_checksum;
-		nvm->ops.update = igb_update_nvm_checksum;
-		nvm->ops.acquire = igb_acquire_nvm_82575;
-		nvm->ops.release = igb_release_nvm_82575;
-		if (nvm->word_size < (1 << 15))
-			nvm->ops.read = igb_read_nvm_eerd;
-		else
-			nvm->ops.read = igb_read_nvm_spi;
-		nvm->ops.write = igb_write_nvm_spi;
-		break;
-	}
+	ret_val = igb_init_nvm_params_82575(hw);
+	if (ret_val)
+		goto out;
 
 	/* if part supports SR-IOV then initialize mailbox parameters */
 	switch (mac->type) {
@@ -638,107 +495,10 @@ static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 	}
 
 	/* setup PHY parameters */
-	if (phy->media_type != e1000_media_type_copper) {
-		phy->type = e1000_phy_none;
-		return 0;
-	}
-
-	phy->autoneg_mask        = AUTONEG_ADVERTISE_SPEED_DEFAULT;
-	phy->reset_delay_us      = 100;
-
-	ctrl_ext = rd32(E1000_CTRL_EXT);
-
-	/* PHY function pointers */
-	if (igb_sgmii_active_82575(hw)) {
-		phy->ops.reset      = igb_phy_hw_reset_sgmii_82575;
-		ctrl_ext |= E1000_CTRL_I2C_ENA;
-	} else {
-		phy->ops.reset      = igb_phy_hw_reset;
-		ctrl_ext &= ~E1000_CTRL_I2C_ENA;
-	}
-
-	wr32(E1000_CTRL_EXT, ctrl_ext);
-	igb_reset_mdicnfg_82580(hw);
-
-	if (igb_sgmii_active_82575(hw) && !igb_sgmii_uses_mdio_82575(hw)) {
-		phy->ops.read_reg   = igb_read_phy_reg_sgmii_82575;
-		phy->ops.write_reg  = igb_write_phy_reg_sgmii_82575;
-	} else if ((hw->mac.type == e1000_82580)
-		|| (hw->mac.type == e1000_i350)) {
-		phy->ops.read_reg   = igb_read_phy_reg_82580;
-		phy->ops.write_reg  = igb_write_phy_reg_82580;
-	} else if (hw->phy.type >= e1000_phy_i210) {
-		phy->ops.read_reg   = igb_read_phy_reg_gs40g;
-		phy->ops.write_reg  = igb_write_phy_reg_gs40g;
-	} else {
-		phy->ops.read_reg   = igb_read_phy_reg_igp;
-		phy->ops.write_reg  = igb_write_phy_reg_igp;
-	}
-
-	/* set lan id */
-	hw->bus.func = (rd32(E1000_STATUS) & E1000_STATUS_FUNC_MASK) >>
-	               E1000_STATUS_FUNC_SHIFT;
+	ret_val = igb_init_phy_params_82575(hw);
 
-	/* Set phy->phy_addr and phy->id. */
-	ret_val = igb_get_phy_id_82575(hw);
-	if (ret_val)
-		return ret_val;
-
-	/* Verify phy id and set remaining function pointers */
-	switch (phy->id) {
-	case I347AT4_E_PHY_ID:
-	case M88E1112_E_PHY_ID:
-	case M88E1111_I_PHY_ID:
-		phy->type                   = e1000_phy_m88;
-		phy->ops.get_phy_info       = igb_get_phy_info_m88;
-
-		if (phy->id == I347AT4_E_PHY_ID ||
-		    phy->id == M88E1112_E_PHY_ID)
-			phy->ops.get_cable_length = igb_get_cable_length_m88_gen2;
-		else
-			phy->ops.get_cable_length = igb_get_cable_length_m88;
-
-		if (phy->id == I210_I_PHY_ID) {
-			phy->ops.get_cable_length =
-					 igb_get_cable_length_m88_gen2;
-			phy->ops.set_d0_lplu_state =
-					igb_set_d0_lplu_state_82580;
-			phy->ops.set_d3_lplu_state =
-					igb_set_d3_lplu_state_82580;
-		}
-		phy->ops.force_speed_duplex = igb_phy_force_speed_duplex_m88;
-		break;
-	case IGP03E1000_E_PHY_ID:
-		phy->type                   = e1000_phy_igp_3;
-		phy->ops.get_phy_info       = igb_get_phy_info_igp;
-		phy->ops.get_cable_length   = igb_get_cable_length_igp_2;
-		phy->ops.force_speed_duplex = igb_phy_force_speed_duplex_igp;
-		phy->ops.set_d0_lplu_state  = igb_set_d0_lplu_state_82575;
-		phy->ops.set_d3_lplu_state  = igb_set_d3_lplu_state;
-		break;
-	case I82580_I_PHY_ID:
-	case I350_I_PHY_ID:
-		phy->type                   = e1000_phy_82580;
-		phy->ops.force_speed_duplex = igb_phy_force_speed_duplex_82580;
-		phy->ops.get_cable_length   = igb_get_cable_length_82580;
-		phy->ops.get_phy_info       = igb_get_phy_info_82580;
-		phy->ops.set_d0_lplu_state  = igb_set_d0_lplu_state_82580;
-		phy->ops.set_d3_lplu_state  = igb_set_d3_lplu_state_82580;
-		break;
-	case I210_I_PHY_ID:
-		phy->type                   = e1000_phy_i210;
-		phy->ops.get_phy_info       = igb_get_phy_info_m88;
-		phy->ops.check_polarity     = igb_check_polarity_m88;
-		phy->ops.get_cable_length   = igb_get_cable_length_m88_gen2;
-		phy->ops.set_d0_lplu_state  = igb_set_d0_lplu_state_82580;
-		phy->ops.set_d3_lplu_state  = igb_set_d3_lplu_state_82580;
-		phy->ops.force_speed_duplex = igb_phy_force_speed_duplex_m88;
-		break;
-	default:
-		return -E1000_ERR_PHY;
-	}
-
-	return 0;
+out:
+	return ret_val;
 }
 
 /**
-- 
1.7.11.7

^ permalink raw reply related

* [net-next 10/10] ixgbe: refactor initialization of feature flags
From: Jeff Kirsher @ 2013-02-15 10:04 UTC (permalink / raw)
  To: davem; +Cc: Emil Tantilov, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1360922649-3388-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Emil Tantilov <emil.s.tantilov@intel.com>

This patch reshuffles the switch/case structure of the flag assignment to
allow for the flags to be set for each MAC type separately. This is needed
for new HW that does not have feature parity with older HW.

Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Tested-by: Jack Morgan <jack.morgan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 58 ++++++++++++++++++---------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 1c0efcb..1d5e093 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -4480,38 +4480,56 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
 	hw->subsystem_vendor_id = pdev->subsystem_vendor;
 	hw->subsystem_device_id = pdev->subsystem_device;
 
-	/* Set capability flags */
+	/* Set common capability flags and settings */
 	rss = min_t(int, IXGBE_MAX_RSS_INDICES, num_online_cpus());
 	adapter->ring_feature[RING_F_RSS].limit = rss;
+	adapter->flags2 |= IXGBE_FLAG2_RSC_CAPABLE;
+	adapter->flags2 |= IXGBE_FLAG2_RSC_ENABLED;
+	adapter->ring_feature[RING_F_FDIR].limit = IXGBE_MAX_FDIR_INDICES;
+	adapter->max_q_vectors = MAX_Q_VECTORS_82599;
+	adapter->atr_sample_rate = 20;
+	adapter->fdir_pballoc = IXGBE_FDIR_PBALLOC_64K;
+#ifdef CONFIG_IXGBE_DCA
+	adapter->flags |= IXGBE_FLAG_DCA_CAPABLE;
+#endif
+#ifdef IXGBE_FCOE
+	adapter->flags |= IXGBE_FLAG_FCOE_CAPABLE;
+	adapter->flags &= ~IXGBE_FLAG_FCOE_ENABLED;
+#ifdef CONFIG_IXGBE_DCB
+	/* Default traffic class to use for FCoE */
+	adapter->fcoe.up = IXGBE_FCOE_DEFTC;
+#endif /* CONFIG_IXGBE_DCB */
+#endif /* IXGBE_FCOE */
+
+	/* Set MAC specific capability flags and exceptions */
 	switch (hw->mac.type) {
 	case ixgbe_mac_82598EB:
+		adapter->flags2 &= ~IXGBE_FLAG2_RSC_CAPABLE;
+		adapter->flags2 &= ~IXGBE_FLAG2_RSC_ENABLED;
+
 		if (hw->device_id == IXGBE_DEV_ID_82598AT)
 			adapter->flags |= IXGBE_FLAG_FAN_FAIL_CAPABLE;
+
 		adapter->max_q_vectors = MAX_Q_VECTORS_82598;
+		adapter->ring_feature[RING_F_FDIR].limit = 0;
+		adapter->atr_sample_rate = 0;
+		adapter->fdir_pballoc = 0;
+#ifdef IXGBE_FCOE
+		adapter->flags &= ~IXGBE_FLAG_FCOE_CAPABLE;
+		adapter->flags &= ~IXGBE_FLAG_FCOE_ENABLED;
+#ifdef CONFIG_IXGBE_DCB
+		adapter->fcoe.up = 0;
+#endif /* IXGBE_DCB */
+#endif /* IXGBE_FCOE */
+		break;
+	case ixgbe_mac_82599EB:
+		if (hw->device_id == IXGBE_DEV_ID_82599_T3_LOM)
+			adapter->flags2 |= IXGBE_FLAG2_TEMP_SENSOR_CAPABLE;
 		break;
 	case ixgbe_mac_X540:
 		fwsm = IXGBE_READ_REG(hw, IXGBE_FWSM);
 		if (fwsm & IXGBE_FWSM_TS_ENABLED)
 			adapter->flags2 |= IXGBE_FLAG2_TEMP_SENSOR_CAPABLE;
-	case ixgbe_mac_82599EB:
-		adapter->max_q_vectors = MAX_Q_VECTORS_82599;
-		adapter->flags2 |= IXGBE_FLAG2_RSC_CAPABLE;
-		adapter->flags2 |= IXGBE_FLAG2_RSC_ENABLED;
-		if (hw->device_id == IXGBE_DEV_ID_82599_T3_LOM)
-			adapter->flags2 |= IXGBE_FLAG2_TEMP_SENSOR_CAPABLE;
-		/* Flow Director hash filters enabled */
-		adapter->atr_sample_rate = 20;
-		adapter->ring_feature[RING_F_FDIR].limit =
-							 IXGBE_MAX_FDIR_INDICES;
-		adapter->fdir_pballoc = IXGBE_FDIR_PBALLOC_64K;
-#ifdef IXGBE_FCOE
-		adapter->flags |= IXGBE_FLAG_FCOE_CAPABLE;
-		adapter->flags &= ~IXGBE_FLAG_FCOE_ENABLED;
-#ifdef CONFIG_IXGBE_DCB
-		/* Default traffic class to use for FCoE */
-		adapter->fcoe.up = IXGBE_FCOE_DEFTC;
-#endif
-#endif /* IXGBE_FCOE */
 		break;
 	default:
 		break;
-- 
1.7.11.7

^ permalink raw reply related

* [net-next 07/10] igb: Initialize NVM function pointers
From: Jeff Kirsher @ 2013-02-15 10:04 UTC (permalink / raw)
  To: davem; +Cc: Akeem G. Abodunrin, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1360922649-3388-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>

This patch initializes NVM function pointers for device configuration.

Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_82575.c | 109 +++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c
index fc69414..e59fc9b 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -223,6 +223,115 @@ out:
 	return ret_val;
 }
 
+/**
+ *  igb_init_nvm_params_82575 - Init NVM func ptrs.
+ *  @hw: pointer to the HW structure
+ **/
+s32 igb_init_nvm_params_82575(struct e1000_hw *hw)
+{
+	struct e1000_nvm_info *nvm = &hw->nvm;
+	u32 eecd = rd32(E1000_EECD);
+	u16 size;
+
+	size = (u16)((eecd & E1000_EECD_SIZE_EX_MASK) >>
+		     E1000_EECD_SIZE_EX_SHIFT);
+	/* Added to a constant, "size" becomes the left-shift value
+	 * for setting word_size.
+	 */
+	size += NVM_WORD_SIZE_BASE_SHIFT;
+
+	/* Just in case size is out of range, cap it to the largest
+	 * EEPROM size supported
+	 */
+	if (size > 15)
+		size = 15;
+
+	nvm->word_size = 1 << size;
+	if (hw->mac.type < e1000_i210) {
+		nvm->opcode_bits = 8;
+		nvm->delay_usec = 1;
+
+		switch (nvm->override) {
+		case e1000_nvm_override_spi_large:
+			nvm->page_size = 32;
+			nvm->address_bits = 16;
+			break;
+		case e1000_nvm_override_spi_small:
+			nvm->page_size = 8;
+			nvm->address_bits = 8;
+			break;
+		default:
+			nvm->page_size = eecd & E1000_EECD_ADDR_BITS ? 32 : 8;
+			nvm->address_bits = eecd & E1000_EECD_ADDR_BITS ?
+					    16 : 8;
+			break;
+		}
+		if (nvm->word_size == (1 << 15))
+			nvm->page_size = 128;
+
+		nvm->type = e1000_nvm_eeprom_spi;
+	} else {
+		nvm->type = e1000_nvm_flash_hw;
+	}
+
+	/* NVM Function Pointers */
+	switch (hw->mac.type) {
+	case e1000_82580:
+		nvm->ops.validate = igb_validate_nvm_checksum_82580;
+		nvm->ops.update = igb_update_nvm_checksum_82580;
+		nvm->ops.acquire = igb_acquire_nvm_82575;
+		nvm->ops.release = igb_release_nvm_82575;
+		if (nvm->word_size < (1 << 15))
+			nvm->ops.read = igb_read_nvm_eerd;
+		else
+			nvm->ops.read = igb_read_nvm_spi;
+		nvm->ops.write = igb_write_nvm_spi;
+		break;
+	case e1000_i350:
+		nvm->ops.validate = igb_validate_nvm_checksum_i350;
+		nvm->ops.update = igb_update_nvm_checksum_i350;
+		nvm->ops.acquire = igb_acquire_nvm_82575;
+		nvm->ops.release = igb_release_nvm_82575;
+		if (nvm->word_size < (1 << 15))
+			nvm->ops.read = igb_read_nvm_eerd;
+		else
+			nvm->ops.read = igb_read_nvm_spi;
+		nvm->ops.write = igb_write_nvm_spi;
+		break;
+	case e1000_i210:
+		nvm->ops.validate = igb_validate_nvm_checksum_i210;
+		nvm->ops.update   = igb_update_nvm_checksum_i210;
+		nvm->ops.acquire = igb_acquire_nvm_i210;
+		nvm->ops.release = igb_release_nvm_i210;
+		nvm->ops.read    = igb_read_nvm_srrd_i210;
+		nvm->ops.write   = igb_write_nvm_srwr_i210;
+		nvm->ops.valid_led_default = igb_valid_led_default_i210;
+		break;
+	case e1000_i211:
+		nvm->ops.acquire  = igb_acquire_nvm_i210;
+		nvm->ops.release  = igb_release_nvm_i210;
+		nvm->ops.read     = igb_read_nvm_i211;
+		nvm->ops.valid_led_default = igb_valid_led_default_i210;
+		nvm->ops.validate = NULL;
+		nvm->ops.update   = NULL;
+		nvm->ops.write    = NULL;
+		break;
+	default:
+		nvm->ops.validate = igb_validate_nvm_checksum;
+		nvm->ops.update = igb_update_nvm_checksum;
+		nvm->ops.acquire = igb_acquire_nvm_82575;
+		nvm->ops.release = igb_release_nvm_82575;
+		if (nvm->word_size < (1 << 15))
+			nvm->ops.read = igb_read_nvm_eerd;
+		else
+			nvm->ops.read = igb_read_nvm_spi;
+		nvm->ops.write = igb_write_nvm_spi;
+		break;
+	}
+
+	return 0;
+}
+
 static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 {
 	struct e1000_phy_info *phy = &hw->phy;
-- 
1.7.11.7

^ permalink raw reply related

* [net-next 06/10] igb: Initialize PHY function pointers
From: Jeff Kirsher @ 2013-02-15 10:04 UTC (permalink / raw)
  To: davem; +Cc: Akeem G. Abodunrin, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1360922649-3388-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>

This patch initializes PHY function pointers for device configuration.

Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_82575.c | 112 +++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c
index 54a7c20..fc69414 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -111,6 +111,118 @@ static bool igb_sgmii_uses_mdio_82575(struct e1000_hw *hw)
 	return ext_mdio;
 }
 
+/**
+ *  igb_init_phy_params_82575 - Init PHY func ptrs.
+ *  @hw: pointer to the HW structure
+ **/
+static s32 igb_init_phy_params_82575(struct e1000_hw *hw)
+{
+	struct e1000_phy_info *phy = &hw->phy;
+	s32 ret_val = 0;
+	u32 ctrl_ext;
+
+	if (hw->phy.media_type != e1000_media_type_copper) {
+		phy->type = e1000_phy_none;
+		goto out;
+	}
+
+	phy->autoneg_mask	= AUTONEG_ADVERTISE_SPEED_DEFAULT;
+	phy->reset_delay_us	= 100;
+
+	ctrl_ext = rd32(E1000_CTRL_EXT);
+
+	if (igb_sgmii_active_82575(hw)) {
+		phy->ops.reset = igb_phy_hw_reset_sgmii_82575;
+		ctrl_ext |= E1000_CTRL_I2C_ENA;
+	} else {
+		phy->ops.reset = igb_phy_hw_reset;
+		ctrl_ext &= ~E1000_CTRL_I2C_ENA;
+	}
+
+	wr32(E1000_CTRL_EXT, ctrl_ext);
+	igb_reset_mdicnfg_82580(hw);
+
+	if (igb_sgmii_active_82575(hw) && !igb_sgmii_uses_mdio_82575(hw)) {
+		phy->ops.read_reg = igb_read_phy_reg_sgmii_82575;
+		phy->ops.write_reg = igb_write_phy_reg_sgmii_82575;
+	} else {
+		switch (hw->mac.type) {
+		case e1000_82580:
+		case e1000_i350:
+			phy->ops.read_reg = igb_read_phy_reg_82580;
+			phy->ops.write_reg = igb_write_phy_reg_82580;
+			break;
+		case e1000_i210:
+		case e1000_i211:
+			phy->ops.read_reg = igb_read_phy_reg_gs40g;
+			phy->ops.write_reg = igb_write_phy_reg_gs40g;
+			break;
+		default:
+			phy->ops.read_reg = igb_read_phy_reg_igp;
+			phy->ops.write_reg = igb_write_phy_reg_igp;
+		}
+	}
+
+	/* set lan id */
+	hw->bus.func = (rd32(E1000_STATUS) & E1000_STATUS_FUNC_MASK) >>
+			E1000_STATUS_FUNC_SHIFT;
+
+	/* Set phy->phy_addr and phy->id. */
+	ret_val = igb_get_phy_id_82575(hw);
+	if (ret_val)
+		return ret_val;
+
+	/* Verify phy id and set remaining function pointers */
+	switch (phy->id) {
+	case I347AT4_E_PHY_ID:
+	case M88E1112_E_PHY_ID:
+	case M88E1111_I_PHY_ID:
+		phy->type		= e1000_phy_m88;
+		phy->ops.get_phy_info	= igb_get_phy_info_m88;
+		if (phy->id == I347AT4_E_PHY_ID ||
+		    phy->id == M88E1112_E_PHY_ID)
+			phy->ops.get_cable_length =
+					 igb_get_cable_length_m88_gen2;
+		else
+			phy->ops.get_cable_length = igb_get_cable_length_m88;
+		phy->ops.force_speed_duplex = igb_phy_force_speed_duplex_m88;
+		break;
+	case IGP03E1000_E_PHY_ID:
+		phy->type = e1000_phy_igp_3;
+		phy->ops.get_phy_info = igb_get_phy_info_igp;
+		phy->ops.get_cable_length = igb_get_cable_length_igp_2;
+		phy->ops.force_speed_duplex = igb_phy_force_speed_duplex_igp;
+		phy->ops.set_d0_lplu_state = igb_set_d0_lplu_state_82575;
+		phy->ops.set_d3_lplu_state = igb_set_d3_lplu_state;
+		break;
+	case I82580_I_PHY_ID:
+	case I350_I_PHY_ID:
+		phy->type = e1000_phy_82580;
+		phy->ops.force_speed_duplex =
+					 igb_phy_force_speed_duplex_82580;
+		phy->ops.get_cable_length = igb_get_cable_length_82580;
+		phy->ops.get_phy_info = igb_get_phy_info_82580;
+		phy->ops.set_d0_lplu_state = igb_set_d0_lplu_state_82580;
+		phy->ops.set_d3_lplu_state = igb_set_d3_lplu_state_82580;
+		break;
+	case I210_I_PHY_ID:
+		phy->type		= e1000_phy_i210;
+		phy->ops.check_polarity	= igb_check_polarity_m88;
+		phy->ops.get_phy_info	= igb_get_phy_info_m88;
+		phy->ops.get_cable_length = igb_get_cable_length_m88_gen2;
+		phy->ops.set_d0_lplu_state = igb_set_d0_lplu_state_82580;
+		phy->ops.set_d3_lplu_state = igb_set_d3_lplu_state_82580;
+		phy->ops.force_speed_duplex = igb_phy_force_speed_duplex_m88;
+		break;
+	default:
+		ret_val = -E1000_ERR_PHY;
+		goto out;
+	}
+
+out:
+	return ret_val;
+}
+
 static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 {
 	struct e1000_phy_info *phy = &hw->phy;
-- 
1.7.11.7

^ permalink raw reply related

* [net-next 08/10] igb: Intialize MAC function pointers
From: Jeff Kirsher @ 2013-02-15 10:04 UTC (permalink / raw)
  To: davem; +Cc: Akeem G. Abodunrin, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1360922649-3388-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>

This patch initializes MAC function pointers for device configuration.

Signed-off-by: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/e1000_82575.c | 61 ++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c
index e59fc9b..8604013 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -332,6 +332,67 @@ s32 igb_init_nvm_params_82575(struct e1000_hw *hw)
 	return 0;
 }
 
+/**
+ *  igb_init_mac_params_82575 - Init MAC func ptrs.
+ *  @hw: pointer to the HW structure
+ **/
+static s32 igb_init_mac_params_82575(struct e1000_hw *hw)
+{
+	struct e1000_mac_info *mac = &hw->mac;
+	struct e1000_dev_spec_82575 *dev_spec = &hw->dev_spec._82575;
+
+	/* Set mta register count */
+	mac->mta_reg_count = 128;
+	/* Set rar entry count */
+	switch (mac->type) {
+	case e1000_82576:
+		mac->rar_entry_count = E1000_RAR_ENTRIES_82576;
+		break;
+	case e1000_82580:
+		mac->rar_entry_count = E1000_RAR_ENTRIES_82580;
+		break;
+	case e1000_i350:
+		mac->rar_entry_count = E1000_RAR_ENTRIES_I350;
+		break;
+	default:
+		mac->rar_entry_count = E1000_RAR_ENTRIES_82575;
+		break;
+	}
+	/* reset */
+	if (mac->type >= e1000_82580)
+		mac->ops.reset_hw = igb_reset_hw_82580;
+	else
+		mac->ops.reset_hw = igb_reset_hw_82575;
+
+	if (mac->type >= e1000_i210) {
+		mac->ops.acquire_swfw_sync = igb_acquire_swfw_sync_i210;
+		mac->ops.release_swfw_sync = igb_release_swfw_sync_i210;
+
+	} else {
+		mac->ops.acquire_swfw_sync = igb_acquire_swfw_sync_82575;
+		mac->ops.release_swfw_sync = igb_release_swfw_sync_82575;
+	}
+
+	/* Set if part includes ASF firmware */
+	mac->asf_firmware_present = true;
+	/* Set if manageability features are enabled. */
+	mac->arc_subsystem_valid =
+		(rd32(E1000_FWSM) & E1000_FWSM_MODE_MASK)
+			? true : false;
+	/* enable EEE on i350 parts and later parts */
+	if (mac->type >= e1000_i350)
+		dev_spec->eee_disable = false;
+	else
+		dev_spec->eee_disable = true;
+	/* physical interface link setup */
+	mac->ops.setup_physical_interface =
+		(hw->phy.media_type == e1000_media_type_copper)
+			? igb_setup_copper_link_82575
+			: igb_setup_serdes_link_82575;
+
+	return 0;
+}
+
 static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 {
 	struct e1000_phy_info *phy = &hw->phy;
-- 
1.7.11.7

^ permalink raw reply related

* [net-next 03/10] igb: Fix for improper allocation flag in igb_get_i2c_client
From: Jeff Kirsher @ 2013-02-15 10:04 UTC (permalink / raw)
  To: davem; +Cc: Carolyn Wyborny, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1360922649-3388-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Carolyn Wyborny <carolyn.wyborny@intel.com>

This patch fixes the allocation function in igb_get_i2c_client to use
GFP_ATOMIC instead of GFP_KERNEL because we have a spinlock.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index d84f28c..c19a35c 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7751,7 +7751,7 @@ igb_get_i2c_client(struct igb_adapter *adapter, u8 dev_addr)
 	}
 
 	/* no client_list found, create a new one */
-	client_list = kzalloc(sizeof(*client_list), GFP_KERNEL);
+	client_list = kzalloc(sizeof(*client_list), GFP_ATOMIC);
 	if (client_list == NULL)
 		goto exit;
 
-- 
1.7.11.7

^ permalink raw reply related

* [net-next v2 05/10] igb: Update igb to use a path similar to ixgbe to determine when to stop Tx
From: Jeff Kirsher @ 2013-02-15 10:04 UTC (permalink / raw)
  To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Eric Dumazet,
	Jeff Kirsher
In-Reply-To: <1360922649-3388-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alexander Duyck <alexander.h.duyck@intel.com>

After reviewing the igb and ixgbe code I realized there are a few issues in
how the code is structured.  Specifically we are not checking the size of the
buffers being used in transmits and we are not using the same value to
determine when to stop or start a Tx queue.  As such the code is prone to be
buggy.

This patch makes it so that we have one value DESC_NEEDED that we will use for
starting and stopping the queue.  In addition we will check the size of
buffers being used when setting up a transmit so as to avoid a possible buffer
overrun if we were to receive a frame with a block of data larger than 32K in
skb->data.

v2: Update to use NETDEV_FRAG_PAGE_MAX_SIZE instead of PAGE_SIZE based
    on feedback from Eric Dumazet

CC: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb.h      | 13 +++++++++++--
 drivers/net/ethernet/intel/igb/igb_main.c | 30 +++++++++++++++++-------------
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index afdb8bb..d27edbc 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -139,8 +139,6 @@ struct vf_data_storage {
 #define IGB_RX_HDR_LEN		IGB_RXBUFFER_256
 #define IGB_RX_BUFSZ		IGB_RXBUFFER_2048
 
-/* How many Tx Descriptors do we need to call netif_wake_queue ? */
-#define IGB_TX_QUEUE_WAKE	16
 /* How many Rx Buffers do we bundle into one write to the hardware ? */
 #define IGB_RX_BUFFER_WRITE	16	/* Must be power of 2 */
 
@@ -169,6 +167,17 @@ enum igb_tx_flags {
 #define IGB_TX_FLAGS_VLAN_MASK		0xffff0000
 #define IGB_TX_FLAGS_VLAN_SHIFT	16
 
+/*
+ * The largest size we can write to the descriptor is 65535.  In order to
+ * maintain a power of two alignment we have to limit ourselves to 32K.
+ */
+#define IGB_MAX_TXD_PWR	15
+#define IGB_MAX_DATA_PER_TXD	(1 << IGB_MAX_TXD_PWR)
+
+/* Tx Descriptors needed, worst case */
+#define TXD_USE_COUNT(S) DIV_ROUND_UP((S), IGB_MAX_DATA_PER_TXD)
+#define DESC_NEEDED (MAX_SKB_FRAGS + 4)
+
 /* wrapper around a pointer to a socket buffer,
  * so a DMA handle can be stored along with the buffer */
 struct igb_tx_buffer {
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index bf55972..ed79a1c 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -4434,13 +4434,6 @@ static void igb_tx_olinfo_status(struct igb_ring *tx_ring,
 	tx_desc->read.olinfo_status = cpu_to_le32(olinfo_status);
 }
 
-/*
- * The largest size we can write to the descriptor is 65535.  In order to
- * maintain a power of two alignment we have to limit ourselves to 32K.
- */
-#define IGB_MAX_TXD_PWR	15
-#define IGB_MAX_DATA_PER_TXD	(1<<IGB_MAX_TXD_PWR)
-
 static void igb_tx_map(struct igb_ring *tx_ring,
 		       struct igb_tx_buffer *first,
 		       const u8 hdr_len)
@@ -4609,15 +4602,25 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
 	struct igb_tx_buffer *first;
 	int tso;
 	u32 tx_flags = 0;
+	u16 count = TXD_USE_COUNT(skb_headlen(skb));
 	__be16 protocol = vlan_get_protocol(skb);
 	u8 hdr_len = 0;
 
-	/* need: 1 descriptor per page,
+	/* need: 1 descriptor per page * PAGE_SIZE/IGB_MAX_DATA_PER_TXD,
+	 *       + 1 desc for skb_headlen/IGB_MAX_DATA_PER_TXD,
 	 *       + 2 desc gap to keep tail from touching head,
-	 *       + 1 desc for skb->data,
 	 *       + 1 desc for context descriptor,
-	 * otherwise try next time */
-	if (igb_maybe_stop_tx(tx_ring, skb_shinfo(skb)->nr_frags + 4)) {
+	 * otherwise try next time
+	 */
+	if (NETDEV_FRAG_PAGE_MAX_SIZE > IGB_MAX_DATA_PER_TXD) {
+		unsigned short f;
+		for (f = 0; f < skb_shinfo(skb)->nr_frags; f++)
+			count += TXD_USE_COUNT(skb_shinfo(skb)->frags[f].size);
+	} else {
+		count += skb_shinfo(skb)->nr_frags;
+	}
+
+	if (igb_maybe_stop_tx(tx_ring, count + 3)) {
 		/* this is a hard error */
 		return NETDEV_TX_BUSY;
 	}
@@ -4659,7 +4662,7 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
 	igb_tx_map(tx_ring, first, hdr_len);
 
 	/* Make sure there is space in the ring for the next send. */
-	igb_maybe_stop_tx(tx_ring, MAX_SKB_FRAGS + 4);
+	igb_maybe_stop_tx(tx_ring, DESC_NEEDED);
 
 	return NETDEV_TX_OK;
 
@@ -6063,9 +6066,10 @@ static bool igb_clean_tx_irq(struct igb_q_vector *q_vector)
 		}
 	}
 
+#define TX_WAKE_THRESHOLD (DESC_NEEDED * 2)
 	if (unlikely(total_packets &&
 		     netif_carrier_ok(tx_ring->netdev) &&
-		     igb_desc_unused(tx_ring) >= IGB_TX_QUEUE_WAKE)) {
+		     igb_desc_unused(tx_ring) >= TX_WAKE_THRESHOLD)) {
 		/* Make sure that anybody stopping the queue after this
 		 * sees the new next_to_clean.
 		 */
-- 
1.7.11.7

^ permalink raw reply related

* [net-next v2 04/10] igb: Refix sparse warning in igb_get_i2c_client
From: Jeff Kirsher @ 2013-02-15 10:04 UTC (permalink / raw)
  To: davem; +Cc: Carolyn Wyborny, netdev, gospo, sassmann, Ben Hutchings,
	Jeff Kirsher
In-Reply-To: <1360922649-3388-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Carolyn Wyborny <carolyn.wyborny@intel.com>

This patch correctly resolves the sparse warnings found with this
function.

v2: revise the fix based on feedback from Ben Hutchings

CC: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index c19a35c..bf55972 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7727,7 +7727,7 @@ static DEFINE_SPINLOCK(i2c_clients_lock);
  *  @adapter: adapter struct
  *  @dev_addr: device address of i2c needed.
  */
-struct i2c_client *
+static struct i2c_client *
 igb_get_i2c_client(struct igb_adapter *adapter, u8 dev_addr)
 {
 	ulong flags;
-- 
1.7.11.7

^ permalink raw reply related

* [net-next 01/10] igb: Support using build_skb in the case that jumbo frames are disabled
From: Jeff Kirsher @ 2013-02-15 10:04 UTC (permalink / raw)
  To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1360922649-3388-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Alexander Duyck <alexander.h.duyck@intel.com>

This change makes it so that we can enable the use of build_skb for cases
where jumbo frames are disabled.  The advantage to this is that we do not
have to perform a memcpy to populate the header and as a result we see a
significant performance improvement.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb.h      |   8 ++
 drivers/net/ethernet/intel/igb/igb_main.c | 187 ++++++++++++++++++++++++------
 2 files changed, 161 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 4b78053..afdb8bb 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -275,10 +275,18 @@ struct igb_q_vector {
 enum e1000_ring_flags_t {
 	IGB_RING_FLAG_RX_SCTP_CSUM,
 	IGB_RING_FLAG_RX_LB_VLAN_BSWAP,
+	IGB_RING_FLAG_RX_BUILD_SKB_ENABLED,
 	IGB_RING_FLAG_TX_CTX_IDX,
 	IGB_RING_FLAG_TX_DETECT_HANG
 };
 
+#define ring_uses_build_skb(ring) \
+	test_bit(IGB_RING_FLAG_RX_BUILD_SKB_ENABLED, &(ring)->flags)
+#define set_ring_build_skb_enabled(ring) \
+	set_bit(IGB_RING_FLAG_RX_BUILD_SKB_ENABLED, &(ring)->flags)
+#define clear_ring_build_skb_enabled(ring) \
+	clear_bit(IGB_RING_FLAG_RX_BUILD_SKB_ENABLED, &(ring)->flags)
+
 #define IGB_TXD_DCMD (E1000_ADVTXD_DCMD_EOP | E1000_ADVTXD_DCMD_RS)
 
 #define IGB_RX_DESC(R, i)	    \
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 1aaf193..b070a97 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -3354,6 +3354,20 @@ void igb_configure_rx_ring(struct igb_adapter *adapter,
 	wr32(E1000_RXDCTL(reg_idx), rxdctl);
 }
 
+static void igb_set_rx_buffer_len(struct igb_adapter *adapter,
+				  struct igb_ring *rx_ring)
+{
+#define IGB_MAX_BUILD_SKB_SIZE \
+	(SKB_WITH_OVERHEAD(IGB_RX_BUFSZ) - \
+	 (NET_SKB_PAD + NET_IP_ALIGN + IGB_TS_HDR_LEN))
+
+	/* set build_skb flag */
+	if (adapter->max_frame_size <= IGB_MAX_BUILD_SKB_SIZE)
+		set_ring_build_skb_enabled(rx_ring);
+	else
+		clear_ring_build_skb_enabled(rx_ring);
+}
+
 /**
  * igb_configure_rx - Configure receive Unit after Reset
  * @adapter: board private structure
@@ -3373,8 +3387,11 @@ static void igb_configure_rx(struct igb_adapter *adapter)
 
 	/* Setup the HW Rx Head and Tail Descriptor Pointers and
 	 * the Base and Length of the Rx Descriptor Ring */
-	for (i = 0; i < adapter->num_rx_queues; i++)
-		igb_configure_rx_ring(adapter, adapter->rx_ring[i]);
+	for (i = 0; i < adapter->num_rx_queues; i++) {
+		struct igb_ring *rx_ring = adapter->rx_ring[i];
+		igb_set_rx_buffer_len(adapter, rx_ring);
+		igb_configure_rx_ring(adapter, rx_ring);
+	}
 }
 
 /**
@@ -6097,6 +6114,41 @@ static void igb_reuse_rx_page(struct igb_ring *rx_ring,
 					 DMA_FROM_DEVICE);
 }
 
+static bool igb_can_reuse_rx_page(struct igb_rx_buffer *rx_buffer,
+				  struct page *page,
+				  unsigned int truesize)
+{
+	/* avoid re-using remote pages */
+	if (unlikely(page_to_nid(page) != numa_node_id()))
+		return false;
+
+#if (PAGE_SIZE < 8192)
+	/* if we are only owner of page we can reuse it */
+	if (unlikely(page_count(page) != 1))
+		return false;
+
+	/* flip page offset to other buffer */
+	rx_buffer->page_offset ^= IGB_RX_BUFSZ;
+
+	/* since we are the only owner of the page and we need to
+	 * increment it, just set the value to 2 in order to avoid
+	 * an unnecessary locked operation
+	 */
+	atomic_set(&page->_count, 2);
+#else
+	/* move offset up to the next cache line */
+	rx_buffer->page_offset += truesize;
+
+	if (rx_buffer->page_offset > (PAGE_SIZE - IGB_RX_BUFSZ))
+		return false;
+
+	/* bump ref count on page before it is given to the stack */
+	get_page(page);
+#endif
+
+	return true;
+}
+
 /**
  * igb_add_rx_frag - Add contents of Rx buffer to sk_buff
  * @rx_ring: rx descriptor ring to transact packets on
@@ -6119,6 +6171,11 @@ static bool igb_add_rx_frag(struct igb_ring *rx_ring,
 {
 	struct page *page = rx_buffer->page;
 	unsigned int size = le16_to_cpu(rx_desc->wb.upper.length);
+#if (PAGE_SIZE < 8192)
+	unsigned int truesize = IGB_RX_BUFSZ;
+#else
+	unsigned int truesize = ALIGN(size, L1_CACHE_BYTES);
+#endif
 
 	if ((size <= IGB_RX_HDR_LEN) && !skb_is_nonlinear(skb)) {
 		unsigned char *va = page_address(page) + rx_buffer->page_offset;
@@ -6141,38 +6198,88 @@ static bool igb_add_rx_frag(struct igb_ring *rx_ring,
 	}
 
 	skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
-			rx_buffer->page_offset, size, IGB_RX_BUFSZ);
+			rx_buffer->page_offset, size, truesize);
 
-	/* avoid re-using remote pages */
-	if (unlikely(page_to_nid(page) != numa_node_id()))
-		return false;
+	return igb_can_reuse_rx_page(rx_buffer, page, truesize);
+}
 
+static struct sk_buff *igb_build_rx_buffer(struct igb_ring *rx_ring,
+					   union e1000_adv_rx_desc *rx_desc)
+{
+	struct igb_rx_buffer *rx_buffer;
+	struct sk_buff *skb;
+	struct page *page;
+	void *page_addr;
+	unsigned int size = le16_to_cpu(rx_desc->wb.upper.length);
 #if (PAGE_SIZE < 8192)
-	/* if we are only owner of page we can reuse it */
-	if (unlikely(page_count(page) != 1))
-		return false;
+	unsigned int truesize = IGB_RX_BUFSZ;
+#else
+	unsigned int truesize = SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) +
+				SKB_DATA_ALIGN(NET_SKB_PAD +
+					       NET_IP_ALIGN +
+					       size);
+#endif
 
-	/* flip page offset to other buffer */
-	rx_buffer->page_offset ^= IGB_RX_BUFSZ;
+	/* If we spanned a buffer we have a huge mess so test for it */
+	BUG_ON(unlikely(!igb_test_staterr(rx_desc, E1000_RXD_STAT_EOP)));
 
-	/*
-	 * since we are the only owner of the page and we need to
-	 * increment it, just set the value to 2 in order to avoid
-	 * an unnecessary locked operation
-	 */
-	atomic_set(&page->_count, 2);
-#else
-	/* move offset up to the next cache line */
-	rx_buffer->page_offset += SKB_DATA_ALIGN(size);
+	/* Guarantee this function can be used by verifying buffer sizes */
+	BUILD_BUG_ON(SKB_WITH_OVERHEAD(IGB_RX_BUFSZ) < (NET_SKB_PAD +
+							NET_IP_ALIGN +
+							IGB_TS_HDR_LEN +
+							ETH_FRAME_LEN +
+							ETH_FCS_LEN));
 
-	if (rx_buffer->page_offset > (PAGE_SIZE - IGB_RX_BUFSZ))
-		return false;
+	rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean];
+	page = rx_buffer->page;
+	prefetchw(page);
 
-	/* bump ref count on page before it is given to the stack */
-	get_page(page);
+	page_addr = page_address(page) + rx_buffer->page_offset;
+
+	/* prefetch first cache line of first page */
+	prefetch(page_addr + NET_SKB_PAD + NET_IP_ALIGN);
+#if L1_CACHE_BYTES < 128
+	prefetch(page_addr + L1_CACHE_BYTES + NET_SKB_PAD + NET_IP_ALIGN);
 #endif
 
-	return true;
+	/* build an skb to around the page buffer */
+	skb = build_skb(page_addr, truesize);
+	if (unlikely(!skb)) {
+		rx_ring->rx_stats.alloc_failed++;
+		return NULL;
+	}
+
+	/* we are reusing so sync this buffer for CPU use */
+	dma_sync_single_range_for_cpu(rx_ring->dev,
+				      rx_buffer->dma,
+				      rx_buffer->page_offset,
+				      IGB_RX_BUFSZ,
+				      DMA_FROM_DEVICE);
+
+	/* update pointers within the skb to store the data */
+	skb_reserve(skb, NET_IP_ALIGN + NET_SKB_PAD);
+	__skb_put(skb, size);
+
+	/* pull timestamp out of packet data */
+	if (igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP)) {
+		igb_ptp_rx_pktstamp(rx_ring->q_vector, skb->data, skb);
+		__skb_pull(skb, IGB_TS_HDR_LEN);
+	}
+
+	if (igb_can_reuse_rx_page(rx_buffer, page, truesize)) {
+		/* hand second half of page back to the ring */
+		igb_reuse_rx_page(rx_ring, rx_buffer);
+	} else {
+		/* we are not reusing the buffer so unmap it */
+		dma_unmap_page(rx_ring->dev, rx_buffer->dma,
+			       PAGE_SIZE, DMA_FROM_DEVICE);
+	}
+
+	/* clear contents of buffer_info */
+	rx_buffer->dma = 0;
+	rx_buffer->page = NULL;
+
+	return skb;
 }
 
 static struct sk_buff *igb_fetch_rx_buffer(struct igb_ring *rx_ring,
@@ -6184,13 +6291,6 @@ static struct sk_buff *igb_fetch_rx_buffer(struct igb_ring *rx_ring,
 
 	rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean];
 
-	/*
-	 * This memory barrier is needed to keep us from reading
-	 * any other fields out of the rx_desc until we know the
-	 * RXD_STAT_DD bit is set
-	 */
-	rmb();
-
 	page = rx_buffer->page;
 	prefetchw(page);
 
@@ -6590,8 +6690,17 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)
 		if (!igb_test_staterr(rx_desc, E1000_RXD_STAT_DD))
 			break;
 
+		/* This memory barrier is needed to keep us from reading
+		 * any other fields out of the rx_desc until we know the
+		 * RXD_STAT_DD bit is set
+		 */
+		rmb();
+
 		/* retrieve a buffer from the ring */
-		skb = igb_fetch_rx_buffer(rx_ring, rx_desc, skb);
+		if (ring_uses_build_skb(rx_ring))
+			skb = igb_build_rx_buffer(rx_ring, rx_desc);
+		else
+			skb = igb_fetch_rx_buffer(rx_ring, rx_desc, skb);
 
 		/* exit if we failed to retrieve a buffer */
 		if (!skb)
@@ -6678,6 +6787,14 @@ static bool igb_alloc_mapped_page(struct igb_ring *rx_ring,
 	return true;
 }
 
+static inline unsigned int igb_rx_offset(struct igb_ring *rx_ring)
+{
+	if (ring_uses_build_skb(rx_ring))
+		return NET_SKB_PAD + NET_IP_ALIGN;
+	else
+		return 0;
+}
+
 /**
  * igb_alloc_rx_buffers - Replace used receive buffers; packet split
  * @adapter: address of board private structure
@@ -6704,7 +6821,9 @@ void igb_alloc_rx_buffers(struct igb_ring *rx_ring, u16 cleaned_count)
 		 * Refresh the desc even if buffer_addrs didn't change
 		 * because each write-back erases this info.
 		 */
-		rx_desc->read.pkt_addr = cpu_to_le64(bi->dma + bi->page_offset);
+		rx_desc->read.pkt_addr = cpu_to_le64(bi->dma +
+						     bi->page_offset +
+						     igb_rx_offset(rx_ring));
 
 		rx_desc++;
 		bi++;
-- 
1.7.11.7

^ permalink raw reply related

* [net-next 02/10] igb: Fix for improper exit in igb_get_i2c_client
From: Jeff Kirsher @ 2013-02-15 10:04 UTC (permalink / raw)
  To: davem; +Cc: Carolyn Wyborny, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1360922649-3388-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Carolyn Wyborny <carolyn.wyborny@intel.com>

This patch fixes an issue where we check for irq's disabled then exit after
explicitly disabling them with spin_lock_irqsave.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
Tested-by: Aaron Brown <arron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index b070a97..d84f28c 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -7750,12 +7750,7 @@ igb_get_i2c_client(struct igb_adapter *adapter, u8 dev_addr)
 		}
 	}
 
-	/* no client_list found, create a new one as long as
-	 * irqs are not disabled
-	 */
-	if (unlikely(irqs_disabled()))
-		goto exit;
-
+	/* no client_list found, create a new one */
 	client_list = kzalloc(sizeof(*client_list), GFP_KERNEL);
 	if (client_list == NULL)
 		goto exit;
-- 
1.7.11.7

^ permalink raw reply related

* [net-next v2 00/10][pull request] Intel Wired LAN Driver Updates 2013.02.15
From: Jeff Kirsher @ 2013-02-15 10:03 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann

This series contains updates to igb and ixgbe.  Most of the changes
are against igb, except for one patch against ixgbe.

There are 3 igb fixes from Carolyn which were reported by Dan
Carpenter which resolve issues found in the get_i2c_client().  Alex
does some cleanup of the igb driver to match similar functionality
in ixgbe on transmit.  Alex also makes it so that we can enable the use
of build_skb for cases where jumbo frames are disabled.  The advantage
to this is that we do not have to perform a memcpy to populate the header
and as a result we see a significant performance improvement.

Akeem provides 4 patches to initialize function pointers and do a
re-factoring of the function pointers in igb_get_variants() to assist
with driver debugging.

The ixgbe patch comes from Emil to reshuffle the switch/case structure
of the flag assignment to allow for the flags to be set for each MAC
type separately. This is needed for new hardware that does not have feature
parity with older hardware.

v2: updated patches 4 & 5 based on feedback from Ben Hutchings and Eric
    Dumazet

The following are changes since commit 9754e293491e3a4e6c1ac020d25140b1ed3d9cd2:
  net: Don't write to current task flags on every packet received.
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Akeem G. Abodunrin (4):
  igb: Initialize PHY function pointers
  igb: Initialize NVM function pointers
  igb: Intialize MAC function pointers
  igb: Refractoring function pointers in igb_get_invariants function

Alexander Duyck (2):
  igb: Support using build_skb in the case that jumbo frames are
    disabled
  igb: Update igb to use a path similar to ixgbe to determine when to
    stop Tx

Carolyn Wyborny (3):
  igb: Fix for improper exit in igb_get_i2c_client
  igb: Fix for improper allocation flag in igb_get_i2c_client
  igb: Refix sparse warning in igb_get_i2c_client

Emil Tantilov (1):
  ixgbe: refactor initialization of feature flags

 drivers/net/ethernet/intel/igb/e1000_82575.c  | 488 ++++++++++++++------------
 drivers/net/ethernet/intel/igb/igb.h          |  21 +-
 drivers/net/ethernet/intel/igb/igb_main.c     | 228 +++++++++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  58 +--
 4 files changed, 495 insertions(+), 300 deletions(-)

-- 
1.7.11.7

^ permalink raw reply

* [PATCH (net.git)] stmmac: fix the parsing of the eee_timer parameter
From: Giuseppe CAVALLARO @ 2013-02-15  9:00 UTC (permalink / raw)
  To: netdev; +Cc: Giuseppe Cavallaro

This pacth fixes the parsing of the eee_timer driver parameter.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b75f4b2..601dd84 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2254,7 +2254,7 @@ static int __init stmmac_cmdline_opt(char *str)
 		} else if (!strncmp(opt, "pause:", 6)) {
 			if (kstrtoint(opt + 6, 0, &pause))
 				goto err;
-		} else if (!strncmp(opt, "eee_timer:", 6)) {
+		} else if (!strncmp(opt, "eee_timer:", 10)) {
 			if (kstrtoint(opt + 10, 0, &eee_timer))
 				goto err;
 		}
-- 
1.7.4.4

^ permalink raw reply related

* Re: [Xen-devel] xen-netback: fix oopes during shutdown and error handling
From: Ian Campbell @ 2013-02-15  8:45 UTC (permalink / raw)
  To: Christopher S. Aker
  Cc: David Vrabel, xen-devel@lists.xen.org, Konrad Rzeszutek Wilk,
	netdev@vger.kernel.org, Jan Beulich, Wei Liu (3P)
In-Reply-To: <511D5DDF.7040000@theshore.net>

On Thu, 2013-02-14 at 21:57 +0000, Christopher S. Aker wrote:
> On 2/14/13 8:18 AM, David Vrabel wrote:
> > These two netback patches fix oopes that may occur during shutdown or
> > if a specific fatal error occurs.
> 
> On 2/14/13 11:39 AM, Ian Campbell wrote:
> > Although I would like to add a Tested-by: Christopher too if
> > possible.
> 
> Tested-by: Christopher S. Aker <caker@theshore.net> (and team)

Thank you!

> We are no longer able to trigger the last OOPs we provided by downing 
> the vif.  Further testing of the other issues will take time, but we're 
> deploying a few patched hosts now.

Great, please let me know how you get on.

Ian.

^ permalink raw reply

* Re: [PATCH] batman-adv: Fix NULL pointer dereference in DAT hash collision avoidance
From: Antonio Quartulli @ 2013-02-15  8:30 UTC (permalink / raw)
  To: Pau Koning; +Cc: davem, netdev, b.a.t.m.a.n
In-Reply-To: <1360664325-7323-1-git-send-email-paukoning@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1406 bytes --]

On Tue, Feb 12, 2013 at 11:18:45AM +0100, Pau Koning wrote:
> An entry in DAT with the hashed position of 0 can cause a NULL pointer
> dereference when the first entry is checked by batadv_choose_next_candidate.
> This first candidate automatically has the max value of 0 and the max_orig_node
> of NULL. Not checking max_orig_node for NULL in batadv_is_orig_node_eligible
> will lead to a NULL pointer dereference when checking for the lowest address.
> 
> This problem was added in 785ea1144182c341b8b85b0f8180291839d176a8
> ("batman-adv: Distributed ARP Table - create DHT helper functions").
> 
> Signed-off-by: Pau Koning <paukoning@gmail.com>


Hello Pau,

thank you very much for this fix, this was not an easy one!

However, next time please CC our mailing list as well (get_maintainer.pl will
give you all the needed addresses), otherwise it may be the case that we
overlook such patches and:
1) we don't review it
2) we don't merge it into our repository (which is where the real development
   goes on).

Both 1) and 2) happened with this patch and, in my humble opinion, it is not a
good idea to merge such delicate fixes without having a reply from the
maintainers.

Therefore, please keep us in the loop when sending patches. It would be really
appreciated.


Regards,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] tcp: fix FIN_WAIT2 timer expression in /proc/net/tcp
From: Toshiaki Makita @ 2013-02-15  8:26 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, netdev
In-Reply-To: <1360682002.13993.3.camel@edumazet-glaptop>

On Tue, 2013-02-12 at 07:13 -0800, Eric Dumazet wrote:
> On Tue, 2013-02-12 at 06:57 -0800, Eric Dumazet wrote:
> 
> > I find this patch confusing :
> > 
> > 1) Please don't change the indentation for a bug fix

OK, I will separate changes of indentation to another patch.

> > 
> > 2) You add a new 'active=3' field, that some user space
> > reading /proc/net/tcp wont expect.

I think, at least, it's harmless and beneficial for netstat.
Even without this change, a keepalive timer of an orphaned
FIN_WAIT2 socket will eventually turn to a timewait timer, 
and by default, or when tcp_fin_timeout is 60, there will be
only timewait timers for orphaned FIN_WAIT2.
If the socket is not closed and SO_KEEPALIVE is set, 
a keepalive timer of FIN_WAIT2 will also be shown in
/proc/net/tcp, whose state isn't equal to above one.
I don't know any application that will be damaged with this
change, and I think the possibility that this change affects
a bad influence to userspace is low.
Please advise.

> > 
> > So the changelog is not matching the changes.
> 
> Also, net/ipv4/inet_diag.c was not changed
> 

thank you for pointing out my mistake.
I will correct it, too.


Toshiaki Makita

^ permalink raw reply

* Re: [tcpdump-workers] [PATCH net 1/2] net: dev_queue_xmit_nit: fix skb->vlan_tci field value
From: Eric W. Biederman @ 2013-02-15  8:17 UTC (permalink / raw)
  To: Paul Pearce
  Cc: Michael Richardson, Eric Dumazet, Ani Sinha, Jiri Pirko, netdev,
	edumazet, tcpdump-workers, dborkman
In-Reply-To: <CAOUgPvSpMmsLpiwmnxwFx9p+B=KOG5VqjNvu_+54hLcBPNK4Cg@mail.gmail.com>

Paul Pearce <pearce@cs.berkeley.edu> writes:

>>> My opinion as a kernel developer is that the network tap is here to have
>>> a copy of the exact frame given to the _device_.
>
>> Good: as someone who spends lots of time with tcpdump doing both network
>> and protocol diagnostics, it's really important to see exactly there.
>> If that means turning off some hardware offload in order to get the
>> intact 1p header, then that may be fine for many situations.
>> (At 10G, on a live router... well...)
>
> I agree as well.
>
> But I think Ani's point was that for RX packets, as of commit
> bcc6d47903612c3861201cc3a866fb604f26b8b2, the filters are not
> getting exactly what's "on the wire." Independent of hardware
> acceleration the vlan headers are being stripped off and skb->vlan_tci
> is being set. That's was the origin of this whole mess.

The mess goes back much farther than that.  That commit just flushed
a lot of the mess out into the open, and made it apparent the kernel
had insufficient facilities for dealing with packets whose vlan
tags had been stripped and that libpcap had not been handling stripped
vlan tags.

> The msg from that commit reads in part:
>> Vlan untagging happens early in __netif_receive_skb so the rest of
>> code (ptype_all handlers, rx_handlers) see the skb like it was
>> untagged by hw.
>
> His confusion (which I share) is why it's acceptable to have this
> behavior of removing headers and setting skb->vlan_tci (regardless of
> hardware acceleration) on the RX path but not also set skb->vlan_tci
> on the TX path.

On all paths the kernel will now set a flag VLAN_TAG_PRESENT if the
vlan_tci is stripped off and used.  So there is no pressing need for a
kernel change.  recvmsg and BPF filters have all of the information they
need to figure out what is going on.  So at this point this is a libpcap
problem not a kernel problem.

On the RX path always stripping the header allowed the vlan processing
code to be simplified and some bugs to be fixed.

Just reading through the code a bit more it looks like stripping the
vlan headers on TX if the network device does not support vlan header
accelleration is a performance loss.  There are other cases besides
AF_PACKET in particular vlan_dev_hard_header that will insert the vlan
header on a packet before the packet is transmitted.

> Indepdent of proposed userspace or PACKET_AUXDATA solutions,
> clarification on the RX skb->vlan_tci behavior would be appreciated.

There are two variables now available in AUXDATA and in the BPF filters
for packets.  VLAN_TAG_PRESENT and VLAN_TAG.

Packets that have their vlan tags stripped have VLAN_TAG_PRESENT set
and the tag is available in VLAN_TAG.

> My knowledge of this code is quite limited so it's entirely possible
> I'm off base here. If so please tell me.

Eric

^ permalink raw reply

* Re: [PATCH] tcp: sysctl to disable TCP simultaneous connect
From: Willy Tarreau @ 2013-02-15  7:55 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Kees Cook, Stephen Hemminger, LKML, Rob Landley, David S. Miller,
	Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, Eric Dumazet, Neil Horman, Yuchung Cheng,
	Shan Wei, linux-doc@vger.kernel.org, netdev
In-Reply-To: <87r4ki6oi1.fsf@xmission.com>

Hi Eric,

On Thu, Feb 14, 2013 at 11:10:46PM -0800, Eric W. Biederman wrote:
> Kees Cook <keescook@chromium.org> writes:
> 
> > On Thu, Feb 14, 2013 at 9:30 PM, Eric W. Biederman
> > <ebiederm@xmission.com> wrote:
> >> Kees Cook <keescook@chromium.org> writes:
> >>
> >>> The patch would not break it -- it defaults the sysctl to staying enabled.
> >>>
> >>> If you mean the documentation should be updated, sure, that's easy to do.
> >>>
> >>> David: I know you aren't a fan of this patch, but I'd like to try to
> >>> convince you. :) This leaves the feature enabled and add a toggle for
> >>> systems (like Chrome OS) that don't want to risk this DoS at all.
> >>> There are so very many other toggle, I don't see why this one would be
> >>> a problem to add.
> >>
> >> Chrome OS has no plans to implement webrtc?  Last I had read that
> >> support had been added to the release versions of Chrome, and was in the
> >> development builds of firefox.  I really don't belive that there are
> >> many systems that don't intend to run a web browser.
> >
> > I haven't looked at the internals of webrtc. Are you implying some
> > feature in it relies on the TCP simultaneous connect?
> 
> I am saying that yes.
> 
> webrtc is built on ICE (interactivity connectivity establishment).  ICE
> support for TCP (RFC6544) uses TCP simultaneous connect.  webrtc
> supports tcp connections.

Then I suspect that a large number of firewalls will need updates after
significant rework for this proposal to succeed. I'm not saying this will
not eventually happen, but there are significant risks associated with
this feature.  Netfilter had this in the window tracking patches around
2002-2003 and this had to be reverted because a client was able to establish
complete connections by sending SYN-SYN/ACK-ACK itself. Other products will
fall through these cracks.

And last but not least, it's the only behaviour in TCP which allows a
random attacker to prevent a connection from establishing by guessing
only a 16-bit port, regardless of any sequence number. Considering how
we've been bothered by people who considered that our sequence numbers
were not random enough, I already expect that the absolute lack of need
of a sequence number will bring new funny articles.

Back then, I did a PoC which permanently prevented an anti-virus proxy
from establishing any connection to its update server, and it did not
require a lot of traffic obviously. People running such proxies probably
don't need webrtc with its assorted lot of issues.

I'm not advocating for pushing the patch, I understand it's not desired.
I just want to ensure that people understand what simultaneous connect
means in terms of DoS for a feature which is rarely used and rarely
technically possible at all.

Regards,
Willy


^ permalink raw reply

* Re: PROBLEM: Software injected vlan tagged packets are unable to be identified using recent BPF modifications
From: Eric W. Biederman @ 2013-02-15  7:20 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Paul Pearce, netdev, tcpdump-workers, davem, edumazet, jpirko,
	Ani Sinha
In-Reply-To: <20130108103811.GA1621@minipsycho.orion>

Jiri Pirko <jiri@resnulli.us> writes:

> Tue, Jan 08, 2013 at 01:05:39AM CET, pearce@cs.berkeley.edu wrote:
>>Hello folks,
>>
>>PROBLEM:
>>
>>vlan tagged packets that are injected via software are not picked up
>>by filters using recent (kernel commit
>>f3335031b9452baebfe49b8b5e55d3fe0c4677d1)
>>BPF vlan modifications. I suspect this is a problem with the Linux
>>kernel.
>>
>>linux-netdev and tcpdump-workers are both cc'd.
>>
>>BACKGROUND:
>>
>>Kernel commit bcc6d47903612c3861201cc3a866fb604f26b8b2 (Jiri
>>Pirko/David S. Miller) removed vlan headers on rx packets prior to
>>them reaching the packet filters. This broke BPF/libpcap's ability to
>>do kernel-level packet filtering based on vlan tag information (the
>>'vlan' keyword).
>>
>>Kernel commit f3335031b9452baebfe49b8b5e55d3fe0c4677d1 (Eric
>>Dumazet/David S. Miller, just merged into Linus's tree
>>http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=f3335031b9452baebfe49b8b5e55d3fe0c4677d1)
>>added the ability to use BPF to once again filter based on vlan
>>tags. Related bpf jit commit:
>>http://www.spinics.net/lists/netdev/msg214759.html
>>
>>libpcap (Ani Sinha) recently RFC'd a patch to use Eric/David's BPF
>>modifications to restore vlan filtering to libpcap.
>>http://www.mail-archive.com/tcpdump-workers@lists.tcpdump.org/msg06810.html
>>I'm using this patch and it works.
>>
>>DETAILS:
>>
>>Under these patches vlan tagged packets received from mediam (actual
>>packets from the wire) can be identified based on vlan tag information
>>using the new BPF functionality.This is good.
>>
>>However, raw vlan tagged packets that are *injected* into the
>>interface using libpcap's pcap_inject() (which is just a fancy wrapper
>>for the send() syscall) are not identified by filters using the recent
>>BPF modifications.
>>
>>The bug manifests itself if you attempt to use the new BPF
>>modifications to filter vlan tagged packets on a live interface. All
>>packets from the medium show up, but all injected packets are dropped.
>>
>>Prior to commit bcc6d47 both medium and injected packets could both be
>>identified using BPFs.
>>
>>These injected packets can however still be identified using the
>>previous, now incorrect "offset into the header" technique. Given
>>this, I suspect what's going on is the kernel code path for these
>>injected packets is not setting skb->vlan_tci correctly (at all?).
>>Since the vlan tag is not in the skb data structure the new BPF
>>modifications don't identify the packets as having a vlan tag,
>>despite it being in the packet header.
>
>
> You are right. skb->vlan_tci is not set. Looking at packet_snd() function
> in net/packet/af_packet.c I guess that something like following patch
> would be needed:
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index e639645..2238559 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2292,6 +2292,12 @@ static int packet_snd(struct socket *sock,
>  	if (unlikely(extra_len == 4))
>  		skb->no_fcs = 1;
>  
> +	if (skb->protocol == cpu_to_be16(ETH_P_8021Q)) {
> +		skb = vlan_untag(skb);
> +		if (unlikely(!skb))
> +			goto out_unlock;
> +	}
> +
>  	/*
>  	 *	Now send it
>  	 */
>
> Thoughts?

That sounds about right.  I don't know how much NIC drivers care but it
looks like af_packet is the only path through the code that can generate
a vlan tagged packet that we will transmit where a vlan tagged packet
can be generated without setting vlan_tci.  So it should make the code
safer.

Certainly we want this to look to the vlan driver like a proper case of
nested vlans and not something weird.

But note we need to handle tpacket_snd as well as packet_snd.

Eric

^ permalink raw reply

* Re: [PATCH] tcp: sysctl to disable TCP simultaneous connect
From: Eric W. Biederman @ 2013-02-15  7:10 UTC (permalink / raw)
  To: Kees Cook
  Cc: Stephen Hemminger, LKML, Rob Landley, David S. Miller,
	Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, Eric Dumazet, Neil Horman, Yuchung Cheng,
	Shan Wei, linux-doc@vger.kernel.org, netdev, Willy Tarreau
In-Reply-To: <CAGXu5jJKQSDo61TD0Jgn9Gq8TL2rCUVbA=Kq4AvfeFUb2FdJMw@mail.gmail.com>

Kees Cook <keescook@chromium.org> writes:

> On Thu, Feb 14, 2013 at 9:30 PM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>> Kees Cook <keescook@chromium.org> writes:
>>
>>> The patch would not break it -- it defaults the sysctl to staying enabled.
>>>
>>> If you mean the documentation should be updated, sure, that's easy to do.
>>>
>>> David: I know you aren't a fan of this patch, but I'd like to try to
>>> convince you. :) This leaves the feature enabled and add a toggle for
>>> systems (like Chrome OS) that don't want to risk this DoS at all.
>>> There are so very many other toggle, I don't see why this one would be
>>> a problem to add.
>>
>> Chrome OS has no plans to implement webrtc?  Last I had read that
>> support had been added to the release versions of Chrome, and was in the
>> development builds of firefox.  I really don't belive that there are
>> many systems that don't intend to run a web browser.
>
> I haven't looked at the internals of webrtc. Are you implying some
> feature in it relies on the TCP simultaneous connect?

I am saying that yes.

webrtc is built on ICE (interactivity connectivity establishment).  ICE
support for TCP (RFC6544) uses TCP simultaneous connect.  webrtc
supports tcp connections.

Eric


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox