Netdev List
 help / color / mirror / Atom feed
* [PATCH 1/2] realtek: convert to per-chip mask
From: Peter Wu @ 2013-08-13 21:25 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, Francois Romieu, lekensteyn
In-Reply-To: <1376429119-31380-1-git-send-email-lekensteyn@gmail.com>

The previous HW_REVID macro did not make identifiers more readable
(compared to hex values like 0x12345678) and only allowed for one static
mask. To make it easier to update the chips list, let's use similar
structures as r8169 and remove HW_REVID.

Names are removed and separated from the table and separated because the
mac_version does not have to be unique.

While at it, change "RTL-xxxx" to "RTLxxxx" to match the names of
Realtek and r8169 driver. Besides that, the only output change is when
a chip is not recognized in which case "TxConfig" is now mentioned
instead of "mask". Since the mask can be anything, the displayed word is
not masked either.

Signed-off-by: Peter Wu <lekensteyn@gmail.com>
---
 realtek.c | 108 ++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 63 insertions(+), 45 deletions(-)

diff --git a/realtek.c b/realtek.c
index c3d7ae5..17b0b7e 100644
--- a/realtek.c
+++ b/realtek.c
@@ -5,13 +5,8 @@
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
-#define HW_REVID(b31, b30, b29, b28, b27, b26, b23, b22) \
-	(b31<<31 | b30<<30 | b29<<29 | b28<<28 | \
-	 b27<<27 | b26<<26 | b23<<23 | b22<<22)
-
 enum chip_type {
-	RTLNONE,
-	RTL8139,
+	RTL8139 = 1,
 	RTL8139_K,
 	RTL8139A,
 	RTL8139A_G,
@@ -22,6 +17,8 @@ enum chip_type {
 	RTL8100B_8139D,
 	RTL8139Cp,
 	RTL8101,
+
+	/* chips not handled by 8139too/8139cp module */
 	RTL8169,
 	RTL8169S,
 	RTL8110S,
@@ -35,36 +32,62 @@ enum chip_type {
 	RTL8100E2,
 };
 
-enum {
-	chip_type_mask = HW_REVID(1, 1, 1, 1, 1, 1, 1, 1)
+static const char * const chip_names[] = {
+	[RTL8139] =		"8139",
+	[RTL8139_K] =		"8139-K",
+	[RTL8139A] =		"8139A",
+	[RTL8139A_G] =		"8139A-G",
+	[RTL8139B] =		"8139B",
+	[RTL8130] =		"8130",
+	[RTL8139C] =		"8139C",
+	[RTL8100] =		"8100",
+	[RTL8100B_8139D] =	"8100B/8139D",
+	[RTL8139Cp] =		"8139C+",
+	[RTL8101] =		"8101",
+
+	/* chips not handled by 8139too/8139cp module */
+	[RTL8169] =		"8169",
+	[RTL8169S] =		"8169S",
+	[RTL8110S] =		"8110S",
+	[RTL8169_8110SB] =	"8169/8110SB",
+	[RTL8169_8110SCd] =	"8169/8110SCd",
+	[RTL8169_8110SCe] =	"8169/8110SCe",
+	[RTL8168_8111Bb] =	"8168/8111Bb",
+	[RTL8168_8111Bef] =	"8168/8111Bef",
+	[RTL8101Ebc] =		"8101Ebc",
+	[RTL8100E1] =		"8100E(1)",
+	[RTL8100E2] =		"8100E(2)",
 };
 
 static struct chip_info {
-	const char *name;
 	u32 id_mask;
+	u32 id_val;
+	int mac_version;
 } rtl_info_tbl[] = {
-	{ "RTL-8139",		HW_REVID(0, 1, 0, 0, 0, 0, 0, 0) },
-	{ "RTL-8139-K",		HW_REVID(0, 1, 1, 0, 0, 0, 0, 0) },
-	{ "RTL-8139A",		HW_REVID(0, 1, 1, 1, 0, 0, 0, 0) },
-	{ "RTL-8139A-G",	HW_REVID(0, 1, 1, 1, 0, 0, 1, 0) },
-	{ "RTL-8139B",		HW_REVID(0, 1, 1, 1, 1, 0, 0, 0) },
-	{ "RTL-8130",		HW_REVID(0, 1, 1, 1, 1, 1, 0, 0) },
-	{ "RTL-8139C",		HW_REVID(0, 1, 1, 1, 0, 1, 0, 0) },
-	{ "RTL-8100",		HW_REVID(0, 1, 1, 1, 1, 0, 1, 0) },
-	{ "RTL-8100B/8139D",	HW_REVID(0, 1, 1, 1, 0, 1, 0, 1) },
-	{ "RTL-8139C+",		HW_REVID(0, 1, 1, 1, 0, 1, 1, 0) },
-	{ "RTL-8101",		HW_REVID(0, 1, 1, 1, 0, 1, 1, 1) },
-	{ "RTL-8169",		HW_REVID(0, 0, 0, 0, 0, 0, 0, 0) },
-	{ "RTL-8169S",		HW_REVID(0, 0, 0, 0, 0, 0, 1, 0) },
-	{ "RTL-8110S",		HW_REVID(0, 0, 0, 0, 0, 1, 0, 0) },
-	{ "RTL-8169/8110SB",	HW_REVID(0, 0, 0, 1, 0, 0, 0, 0) },
-	{ "RTL-8169/8110SCd",	HW_REVID(0, 0, 0, 1, 1, 0, 0, 0) },
-	{ "RTL-8169/8110SCe",	HW_REVID(1, 0, 0, 1, 1, 0, 0, 0) },
-	{ "RTL-8168/8111Bb",	HW_REVID(0, 0, 1, 1, 0, 0, 0, 0) },
-	{ "RTL-8168/8111Bef",	HW_REVID(0, 0, 1, 1, 1, 0, 0, 0) },
-	{ "RTL-8101Ebc",	HW_REVID(0, 0, 1, 1, 0, 1, 0, 0) },
-	{ "RTL-8100E(1)",	HW_REVID(0, 0, 1, 1, 0, 0, 1, 0) },
-	{ "RTL-8100E(2)",	HW_REVID(0, 0, 1, 1, 1, 0, 1, 0) },
+	{ 0xfcc00000, 0x40000000,	RTL8139 },
+	{ 0xfcc00000, 0x60000000,	RTL8139_K },
+	{ 0xfcc00000, 0x70000000,	RTL8139A },
+	{ 0xfcc00000, 0x70800000,	RTL8139A_G },
+	{ 0xfcc00000, 0x78000000,	RTL8139B },
+	{ 0xfcc00000, 0x7c000000,	RTL8130 },
+	{ 0xfcc00000, 0x74000000,	RTL8139C },
+	{ 0xfcc00000, 0x78800000,	RTL8100 },
+	{ 0xfcc00000, 0x74400000,	RTL8100B_8139D },
+	{ 0xfcc00000, 0x74800000,	RTL8139Cp },
+	{ 0xfcc00000, 0x74c00000,	RTL8101 },
+
+	/* chips not handled by 8139too/8139cp module */
+	{ 0xfcc00000, 0x00000000,	RTL8169 },
+	{ 0xfcc00000, 0x00800000,	RTL8169S },
+	{ 0xfcc00000, 0x04000000,	RTL8110S },
+	{ 0xfcc00000, 0x10000000,	RTL8169_8110SB },
+	{ 0xfcc00000, 0x18000000,	RTL8169_8110SCd },
+	{ 0xfcc00000, 0x98000000,	RTL8169_8110SCe },
+	{ 0xfcc00000, 0x30000000,	RTL8168_8111Bb },
+	{ 0xfcc00000, 0x38000000,	RTL8168_8111Bef },
+	{ 0xfcc00000, 0x34000000,	RTL8101Ebc },
+	{ 0xfcc00000, 0x30800000,	RTL8100E1 },
+	{ 0xfcc00000, 0x38800000,	RTL8100E2 },
 	{ }
 };
 
@@ -93,31 +116,26 @@ realtek_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
 	u8 *data8 = (u8 *) regs->data;
 	u32 v;
 	struct chip_info *ci;
-	unsigned int board_type = RTLNONE, i;
+	unsigned int board_type;
 
-	v = data[0x40 >> 2] & chip_type_mask;
+	v = data[0x40 >> 2]; /* TxConfig */
 
 	ci = &rtl_info_tbl[0];
-	while (ci->name) {
-		if (v == ci->id_mask)
+	while (ci->mac_version) {
+		if ((v & ci->id_mask) == ci->id_val)
 			break;
 		ci++;
 	}
-	if (v != ci->id_mask) {
-		fprintf(stderr, "Unknown RealTek chip (mask: 0x%08x)\n", v);
+	board_type = ci->mac_version;
+	if (!board_type) {
+		fprintf(stderr, "Unknown RealTek chip (TxConfig: 0x%08x)\n", v);
 		return 91;
 	}
-	for (i = 0; i < ARRAY_SIZE(rtl_info_tbl); i++) {
-		if (ci == &rtl_info_tbl[i])
-			board_type = i + 1;
-	}
-	if (board_type == RTLNONE)
-		abort();
 
 	fprintf(stdout,
-		"RealTek %s registers:\n"
+		"RealTek RTL%s registers:\n"
 		"--------------------------------------------------------\n",
-		ci->name);
+		chip_names[board_type]);
 
 	fprintf(stdout,
 		"0x00: MAC Address                      %02x:%02x:%02x:%02x:%02x:%02x\n",
-- 
1.8.3.4

^ permalink raw reply related

* [PATCH v4 ethtool 0/2] Add new Realtek devices
From: Peter Wu @ 2013-08-13 21:25 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, Francois Romieu, lekensteyn

Hi,

The list of devices supported by `ethtool -d eth0` was quite outdated
and did not support my onboard NIC. With the following two patches, the
supported devices list will be in sync with the r8169 kernel driver
(r8169 as of 3.11). Note that no new registers have been added, I am
sure that some registers are incorrect (like the Power Management wakeup
frames), but important information such as MAC address is still correct.

This fourth revision corrects the name RTL8169C ("RTL8169C+") to
RTL8169Cp and corrects a wrongly changed identifier for the
RTL8169_8110SCe. (noticed by Ben Hutchings). The last mistake would not
affect the final result because it is changed in the second patch
("realtek: update devices to 3.11").

Regards,
Peter
--

Peter Wu (2):
  realtek: convert to per-chip mask
  realtek: update devices to 3.11

 realtek.c | 285 +++++++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 218 insertions(+), 67 deletions(-)

-- 
1.8.3.4

^ permalink raw reply

* Re: r8169 OOPSen in rtl_rx
From: Francois Romieu @ 2013-08-13 21:15 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: nic_swsd, netdev
In-Reply-To: <20130813094314.GW3008@twins.programming.kicks-ass.net>

Peter Zijlstra <peterz@infradead.org> :
[...]
> I've got an AMD x86_64 machine with two realtek NICs:
> 
> 01:08.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8169 Gigabit Ethernet (rev 10)
> 03:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI Express Gigabit Ethernet controller (rev 06)

Which XID (see kernel dmesg) ?

[...]
> So all I've got to offer currently is a partial backtrace -- see
> attached image.

(no attachment)

> Partial transcribe:
> 
>   ? rtl8169_try_rx_copy.isra.77

/me scratches head.

You may check that pkt_size is > 0, <= ETH_FRAME_LEN (no jumbo ?) and
see if it helps.

-- 
Ueimor

^ permalink raw reply

* Re: [PATCH 2/2] net: asix: Move declaration of ax88172a_info to shared header
From: David Miller @ 2013-08-13 21:14 UTC (permalink / raw)
  To: broonie; +Cc: linux-usb, netdev, linaro-kernel, broonie
In-Reply-To: <1376069482-16951-2-git-send-email-broonie@kernel.org>

From: Mark Brown <broonie@kernel.org>
Date: Fri,  9 Aug 2013 18:31:22 +0100

> From: Mark Brown <broonie@linaro.org>
> 
> Ensure that the definition of ax88172a_info matches the declaration seen
> by users and silence sparse warnings about symbols without declarations
> in the global namespace by moving the declaration into the shared header
> asix.h.
> 
> Signed-off-by: Mark Brown <broonie@linaro.org>

Applied.

^ permalink raw reply

* Re: [PATCH 1/2] net: asix: Staticise non-exported symbols
From: David Miller @ 2013-08-13 21:14 UTC (permalink / raw)
  To: broonie; +Cc: linux-usb, netdev, linaro-kernel, broonie
In-Reply-To: <1376069482-16951-1-git-send-email-broonie@kernel.org>

From: Mark Brown <broonie@kernel.org>
Date: Fri,  9 Aug 2013 18:31:21 +0100

> From: Mark Brown <broonie@linaro.org>
> 
> Make functions that are only referenced from ops structures static, they
> do not need to be in the global namespace and sparse complains about this.
> 
> Signed-off-by: Mark Brown <broonie@linaro.org>

Applied.

^ permalink raw reply

* Find vlanid with non-root user
From: Amador Pahim @ 2013-08-13 20:46 UTC (permalink / raw)
  To: netdev

Hello,

Is there a way to get vlanid from a vlan interface as a regular user?
Default permissions to /proc/net/vlan/ is 600.
-rw-------. 1 root root 0 Aug 13 17:23 /proc/net/vlan/eth1.10-fcoe

Not sure if deducting vlanid from vlan name is a reasonable way to do so.

Thank you in advance,.

Best Regards,
-- 
Pahim

^ permalink raw reply

* Re: question about netif_rx
From: Francois Romieu @ 2013-08-13 20:41 UTC (permalink / raw)
  To: David Shwatrz
  Cc: Julia Lawall, grant.likely, rob.herring, netdev, linux-kernel,
	devicetree
In-Reply-To: <CAJJAcocQ59Kc9a1b=jrNSc1akLLN_74yKANMEmLVmvxoRjX4hw@mail.gmail.com>

(no top-post nor lazy quote please)

David Shwatrz <dshwatrz@gmail.com> :
[...]
> In the napi_gro_receive() we check that the device supports
> NETIF_F_GRO, but I don't see that we inspect checksum or that
> NETIF_F_GRO is depends on checksum.

napi_gro_receive is irrelevant. Let aside tunnel, the real work happens
in the protocol specific gro_receive handlers.

However I am an happy retard and I missed that tcp gro stopped depending
on Rx checksum since commit 861b650101eb0c627d171eb18de81dddb93d395e. :o/

So, yes, napi_gro_receive could be used.

-- 
Ueimor

^ permalink raw reply

* [PATCH] r8169: fix invalid register dump
From: Peter Wu @ 2013-08-13 20:37 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev, nic_swsd, lekensteyn

From: Peter Wu <lekensteyn@gmail.com>

For some reason, my PCIe RTL8111E onboard NIC on a GA-Z68X-UD3H-B3
motherboard reads as FFs when reading from MMIO with a block size
larger than 7. Therefore change to reading blocks of four bytes.

Signed-off-by: Peter Wu <lekensteyn@gmail.com>
---
Hi,

Personally I dislike the repetition of this patch, a hypothetical
`memcpy_fromio_block(bytes, tp->mmio_addr, 4)` would look cleaner, but
perhaps I am doing something wrong because apparantly nobody had a need
for reading multiple blocks from IO?

Anyway, this patch (with patched ethtool) allows `ethtool -d` to be more
useful for the mentioned chip.

Regards,
Peter
---
 drivers/net/ethernet/realtek/r8169.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index b5eb419..28af01c 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1897,12 +1897,17 @@ static void rtl8169_get_regs(struct net_device *dev, struct ethtool_regs *regs,
 			     void *p)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
+	char *bytes = p;
+	int i;
 
 	if (regs->len > R8169_REGS_SIZE)
 		regs->len = R8169_REGS_SIZE;
 
 	rtl_lock_work(tp);
-	memcpy_fromio(p, tp->mmio_addr, regs->len);
+	for (i = 0; i < regs->len - 4; i += 4)
+		memcpy_fromio(bytes + i, tp->mmio_addr + i, 4);
+	if (i < regs->len)
+		memcpy_fromio(bytes + i, tp->mmio_addr + i, regs->len - i);
 	rtl_unlock_work(tp);
 }
 
-- 
1.8.3.4

^ permalink raw reply related

* [PATCH] r8169: remember WOL preferences on driver load
From: Peter Wu @ 2013-08-13 20:19 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev, nic_swsd, lekensteyn

Do not clear Broadcast/Multicast/Unicast Wake Flag or LanWake in
Config5. This is necessary to preserve WOL state when the driver is
loaded. The r8168 vendor driver used to write to both Config1 and
Config5, but in recent versions, this is commented out. Here we keep
writing PMEnable to Config1 because there may be older chips where
PMEnable is not sticky.

Signed-off-by: Peter Wu <lekensteyn@gmail.com>
---
 drivers/net/ethernet/realtek/r8169.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 28af01c..7bdf322 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7093,7 +7093,6 @@ rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	RTL_W8(Cfg9346, Cfg9346_Unlock);
 	RTL_W8(Config1, RTL_R8(Config1) | PMEnable);
-	RTL_W8(Config5, RTL_R8(Config5) & PMEStatus);
 	if ((RTL_R8(Config3) & (LinkUp | MagicPacket)) != 0)
 		tp->features |= RTL_FEATURE_WOL;
 	if ((RTL_R8(Config5) & (UWF | BWF | MWF)) != 0)
-- 
1.8.3.4

^ permalink raw reply related

* [PATCH net] net: tg3: fix NULL pointer dereference in tg3_io_error_detected and tg3_io_slot_reset
From: Nithin Nayak Sujir @ 2013-08-13 18:45 UTC (permalink / raw)
  To: davem; +Cc: netdev, Daniel Borkmann, Gavin Shan, Michael Chan,
	Nithin Nayak Sujir

From: Daniel Borkmann <dborkman@redhat.com>

Commit d8af4dfd8 ("net/tg3: Fix kernel crash") introduced a possible
NULL pointer dereference in tg3 driver when !netdev || !netif_running(netdev)
condition is met and netdev is NULL. Then, the jump to the 'done' label
calls dev_close() with a netdevice that is NULL. Therefore, only call
dev_close() when we have a netdevice, but one that is not running.

[ Add the same checks in tg3_io_slot_reset() per Gavin Shan - by Nithin
Nayak Sujir ]

Reported-by: Dave Jones <davej@redhat.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Gavin Shan <shangw@linux.vnet.ibm.com>
Cc: Michael Chan <mchan@broadcom.com>
Signed-off-by: Nithin Nayak Sujir <nsujir@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index ddebc7a..0da2214 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -17796,8 +17796,10 @@ static pci_ers_result_t tg3_io_error_detected(struct pci_dev *pdev,
 
 done:
 	if (state == pci_channel_io_perm_failure) {
-		tg3_napi_enable(tp);
-		dev_close(netdev);
+		if (netdev) {
+			tg3_napi_enable(tp);
+			dev_close(netdev);
+		}
 		err = PCI_ERS_RESULT_DISCONNECT;
 	} else {
 		pci_disable_device(pdev);
@@ -17827,7 +17829,8 @@ static pci_ers_result_t tg3_io_slot_reset(struct pci_dev *pdev)
 	rtnl_lock();
 
 	if (pci_enable_device(pdev)) {
-		netdev_err(netdev, "Cannot re-enable PCI device after reset.\n");
+		dev_err(&pdev->dev,
+			"Cannot re-enable PCI device after reset.\n");
 		goto done;
 	}
 
@@ -17835,7 +17838,7 @@ static pci_ers_result_t tg3_io_slot_reset(struct pci_dev *pdev)
 	pci_restore_state(pdev);
 	pci_save_state(pdev);
 
-	if (!netif_running(netdev)) {
+	if (!netdev || !netif_running(netdev)) {
 		rc = PCI_ERS_RESULT_RECOVERED;
 		goto done;
 	}
@@ -17847,7 +17850,7 @@ static pci_ers_result_t tg3_io_slot_reset(struct pci_dev *pdev)
 	rc = PCI_ERS_RESULT_RECOVERED;
 
 done:
-	if (rc != PCI_ERS_RESULT_RECOVERED && netif_running(netdev)) {
+	if (rc != PCI_ERS_RESULT_RECOVERED && netdev && netif_running(netdev)) {
 		tg3_napi_enable(tp);
 		dev_close(netdev);
 	}
-- 
1.8.1.4

^ permalink raw reply related

* Re: [PATCH net-next] bonding: lacp_port_id setting for 802.3ad
From: Jay Vosburgh @ 2013-08-13 18:25 UTC (permalink / raw)
  To: Krisztian Ivancso; +Cc: Ding Tianhong, netdev
In-Reply-To: <520A11CD.8060400@ivancso.net>

Krisztian Ivancso <github-ivan@ivancso.net> wrote:

>On 08/13/2013 11:39 AM, Ding Tianhong wrote:
>> On 2013/8/13 17:20, Krisztian Ivancso wrote:
>>> On 08/13/2013 03:07 AM, Ding Tianhong wrote:
>>>> On 2013/8/12 19:19, Krisztian Ivancso wrote:
>>>>> >From 472fffa5a8f170daed9e4cc677af8e2560b86be2 Mon Sep 17 00:00:00 2001
>>>>> From: Krisztian Ivancso <github-ivan@ivancso.net>
>>>>> Date: Sun, 11 Aug 2013 20:30:44 +0200
>>>>> Subject: [PATCH net-next] bonding: lacp_port_id setting for 802.3ad ports
>> 
>> ok, for example: the bonding has four slave, slave1 and slave2 aggregation to 1 group,
>> and slave3 and slave4 aggregtion to 2 group, how you distinguish the 1 and 2 group by initialize id.
>
>this is not possible, because all slave have to be a member of the same
>aggregation group.

	Just on the above point, bonding can group slaves into multiple
aggregators, but only one aggregator will be active at any given time.

	To answer the question, the four slaves would each be given
unique port IDs that do not conflict.

>i think we misunderstood each other.
>
>here is a new example:
>- switch1 is a switch with a configured lag with two members ports
>  (member1 and member2)
>- two linux (linux1 and linux2) box with a configured bonding device
>  (bond0) with the same MAC set in both box and one
>  slave on each
>- lacp_port_id is set to 10 in linux1 and 20 in linux2
>
>you can attach the slave from both linux boxes to the same
>lag on switch1. (slave from linux1 to port member1 and
>slave from linux2 to port member2 on switch1)
>
>port id must be unique within a system.
>bonding implementation set a unique system id for every bonding device
>which is derived from MAC of one of the slave interfaces.
>
>if we use the current bonding implementation second linux box can't be
>a member on switch1 because port id is 1 in both linux bonding device.
>
>if we can set different starting port id for bonding in different boxes
>the second box can be a member also.

	I understand what you're trying to do here (permit multiple
instances of bonding on different systems to connect to a single
aggregator on a switch), and I don't really have a problem with it in
general.

	I do have some comments:

	First, altering the lacp_port_id (via sysfs) should only be
permitted when there are no slaves in the bond, otherwise the
/proc/net/bonding/bond0 output for the first port id will not match the
actual first port id value assigned to the slaves.  As a practical
matter, altering lacp_port_id while slaves are present in the bond has
no effect until all slaves are released and the first new slave is
added, so this is not reducing functionality.

	Second, the lacp_port_id is global across all bonds created
within the loaded module, and so multiple bonds will all use the same
starting value.  Setting the lacp_port_id via sysfs has no effect, as it
alters a per-bond value, bond->params.lacp_port_id, that is never
actually used to set the port ID of a first slave in bond_enslave.

	The global default value should only be used to initialize the
per-bond value when a bond is created, and that per-bond value should be
used when setting the port id in bond_enslave().  The per-bond value is
already displayed in /proc/net/bonding/bond0, and is the value modified
by the sysfs functions

	Third, consider adding the port ID to the 803.2ad section in
bond_info_show_slave.

	Lastly, I think this should be tested against systems other than
Cisco to insure that it really interoperates with, for example,
Juniper's methodology for spanning an aggregator across physical
chassis.  I'm not sure why it wouldn't, but once new functionality
becomes part of the kernel, changing it in non-backwards compatible ways
is difficult.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Sergei Shtylyov @ 2013-08-13 18:05 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, mst
In-Reply-To: <1376416558-25015-1-git-send-email-vyasevic@redhat.com>

Hello.

On 08/13/2013 09:55 PM, Vlad Yasevich wrote:

> When the user turns off IFF_VNET_HDR flag, attempts to change
> offload features via TUNSETOFFLOAD do not work.  This could cause
> GSO packets to be delivered to the user when the user is
> not prepared to handle them.

> To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
> disabled and make sure to turn off all offloads in this case.
> Also, when IFF_VNET_HDR is disabled, run throught the offload change
> as well to make sure that the functionality is not dependent on
> the order of the ioclt() calls.

> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>   drivers/net/macvtap.c | 27 +++++++++++++++++++--------
>   1 file changed, 19 insertions(+), 8 deletions(-)

> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index a98fb0e..076b9e7 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
[...]
> @@ -1092,8 +1097,18 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
>   		if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
>   		    (IFF_NO_PI | IFF_TAP))
>   			ret = -EINVAL;
> -		else
> +		else {
> +			if ((q->flags ^ u) & IFF_VNET_HDR) {
> +				/* vnet_hdr support impacts the offloads,
> +				 * so we need to run throught the offload
> +				 * change.
> +				 */
> +				rtnl_lock();
> +				ret = set_offload(q, 0);
> +				rtnl_unlock();
> +			}
>   			q->flags = u;
> +		}

    According to Documentation/CodingStyle, both arms of the *if* statement 
should have {} if one arm has it.

WBR, Sergei

^ permalink raw reply

* Re: [PATCH net] bnx2x: guard stats_pending with stats_lock in all cases
From: Neal Cardwell @ 2013-08-13 17:57 UTC (permalink / raw)
  To: Dmitry Kravkov
  Cc: David Miller, netdev@vger.kernel.org, Eilon Greenstein,
	Vladislav Zolotarov, Yaniv Rosner, Merav Sicron, Eric Dumazet,
	Tom Herbert, Havard Skinnemoen, Sanjay Hortikar, Dmitry Kravkov
In-Reply-To: <CAM8tLiPUO2EHMM7AZJrj=LfZqCBnpKMbHT9wWomBAp6v_18YYg@mail.gmail.com>

On Mon, Aug 12, 2013 at 4:52 PM, Dmitry Kravkov <dkravkov@gmail.com> wrote:
> Neal, thanks for looking into this.
>
> There's another race, where two flows
> stats_update() and stats_start() may send two queries in parallel
> which will cause FW to assert.
>
> I'm preparing the set for net, which includes the fix  for this race
> also(will be out in a couple of hours)
>
> I will be more than happy if you will take a look.
>
> Thanks
> Dmitry

Thanks! Your "bnx2x: protect different statistics flows" patch looks
like a superset of the synchronization I was proposing in this patch,
so we can drop mine.

Thanks,
neal

^ permalink raw reply

* Re: [PATCH net v5 1/5] bnx2x: protect different statistics flows
From: Stephen Hemminger @ 2013-08-13 17:57 UTC (permalink / raw)
  To: David Miller; +Cc: dmitry, netdev, eilong, ariele
In-Reply-To: <20130812.212402.138911128487906526.davem@davemloft.net>

On Mon, 12 Aug 2013 21:24:02 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Mon, 12 Aug 2013 17:12:08 -0700
> 
> > On Tue, 13 Aug 2013 02:24:59 +0300
> > "Dmitry Kravkov" <dmitry@broadcom.com> wrote:
> > 
> >> +	bool					stats_started;
> >> +	struct semaphore			stats_sema;
> > 
> > Is there a reason to use a counting semaphore? Do you expect it to
> > be held across user process boundary? or want count > 1?
> > 
> > Use of semaphores as a locking primitive is discouraged,
> > instead us a mutex.
> 
> Would you please look at the feedback I gave these guys to the
> previous iteration of these changes?
> 
> They were using custom locking primitives and semaphores gave
> the best approximation to the semantics they were looking for.

Your right in this case sempahore makes sense because it is
used to hold off process while hardware responds.

^ permalink raw reply

* [PATCH] macvtap: Correctly set tap features when IFF_VNET_HDR is disabled.
From: Vlad Yasevich @ 2013-08-13 17:55 UTC (permalink / raw)
  To: netdev; +Cc: mst, Vlad Yasevich

When the user turns off IFF_VNET_HDR flag, attempts to change
offload features via TUNSETOFFLOAD do not work.  This could cause
GSO packets to be delivered to the user when the user is
not prepared to handle them.

To solve, allow processing of TUNSETOFFLOAD when IFF_VNET_HDR is
disabled and make sure to turn off all offloads in this case.
Also, when IFF_VNET_HDR is disabled, run throught the offload change
as well to make sure that the functionality is not dependent on
the order of the ioclt() calls.

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
 drivers/net/macvtap.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a98fb0e..076b9e7 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -1019,6 +1019,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
 	struct macvlan_dev *vlan;
 	netdev_features_t features;
 	netdev_features_t feature_mask = 0;
+	netdev_features_t tap_mask = TUN_OFFLOADS;
 
 	vlan = rtnl_dereference(q->vlan);
 	if (!vlan)
@@ -1026,7 +1027,12 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
 
 	features = vlan->dev->features;
 
-	if (arg & TUN_F_CSUM) {
+	if (!(q->flags & IFF_VNET_HDR)) {
+		/* Turn off all checsum offloading also if user does
+		 * not user vnet_hdr.
+		 */
+		tap_mask |= NETIF_F_ALL_CSUM;
+	} else if (arg & TUN_F_CSUM) {
 		feature_mask = NETIF_F_HW_CSUM;
 
 		if (arg & (TUN_F_TSO4 | TUN_F_TSO6)) {
@@ -1058,8 +1064,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
 	/* tap_features are the same as features on tun/tap and
 	 * reflect user expectations.
 	 */
-	vlan->tap_features = vlan->dev->features &
-			    (feature_mask | ~TUN_OFFLOADS);
+	vlan->tap_features = vlan->dev->features & (feature_mask | ~tap_mask);
 	vlan->set_features = features;
 	netdev_update_features(vlan->dev);
 
@@ -1092,8 +1097,18 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 		if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
 		    (IFF_NO_PI | IFF_TAP))
 			ret = -EINVAL;
-		else
+		else {
+			if ((q->flags ^ u) & IFF_VNET_HDR) {
+				/* vnet_hdr support impacts the offloads,
+				 * so we need to run throught the offload
+				 * change.
+				 */
+				rtnl_lock();
+				ret = set_offload(q, 0);
+				rtnl_unlock();
+			}
 			q->flags = u;
+		}
 
 		return ret;
 
@@ -1155,10 +1170,6 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
 			    TUN_F_TSO_ECN | TUN_F_UFO))
 			return -EINVAL;
 
-		/* TODO: only accept frames with the features that
-			 got enabled for forwarded frames */
-		if (!(q->flags & IFF_VNET_HDR))
-			return  -EINVAL;
 		rtnl_lock();
 		ret = set_offload(q, arg);
 		rtnl_unlock();
-- 
1.8.1.4

^ permalink raw reply related

* Re: [PATCH 2/7] uio: Allow to create custom UIO attributes
From: Greg Kroah-Hartman @ 2013-08-13 17:54 UTC (permalink / raw)
  To: Benedikt Spranger
  Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior, Hans J. Koch,
	Holger Dengler
In-Reply-To: <1376384922-8519-4-git-send-email-b.spranger@linutronix.de>

On Tue, Aug 13, 2013 at 11:08:37AM +0200, Benedikt Spranger wrote:
> This patch the struct uio_attribute which represents a custom UIO
> attribute. The non-standard attributes are stored in a "attr" directory.
> This will be used by the flexcard driver which creates a UIO device that
> is using the "uio_pdrv" and requires one additional value to be read /
> written by the user.
> 
> Cc: "Hans J. Koch" <hjk@hansjkoch.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> Signed-off-by: Holger Dengler <dengler@linutronix.de>
> ---
>  drivers/uio/uio.c          | 106 ++++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/uio_driver.h |  36 +++++++++++----
>  2 files changed, 133 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index 9a95220..d66784a 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -39,6 +39,7 @@ struct uio_device {
>  	struct uio_info		*info;
>  	struct kobject		*map_dir;
>  	struct kobject		*portio_dir;
> +	struct kobject		attr_dir;

If you embed a kobject into a structure, it is now in charge of the
memory reference counting for that object.

Which you did not do correctly:

> +static struct kobj_type uio_attr_type = {
> +	.sysfs_ops	= &uio_sysfs_ops,
> +};

As per the documentation in the kernel, I now get to make fun of the
fact that you obviously didn't test this code (hint, the kernel would
have told you bad things happened if you did...)

But I don't understand your main goal of a new kobject for attributes,
why?  What problem are you trying to solve here that a new kobject, and
new sysfs files attached to the UIO object are going to solve?

>  /**
> + * uio_add_user_attributes - add an extra UIO attribute
> + * @info: UIO device capabilities
> + */
> +static int uio_add_user_attributes(struct uio_info *info)
> +{
> +	struct uio_device *idev = info->uio_dev;
> +	const struct uio_attribute **uio_attr;
> +	int i;
> +	int ret = 0;
> +
> +	uio_attr = info->attributes;
> +	if (!uio_attr)
> +		return 0;
> +
> +	for (i = 0; uio_attr[i]; i++) {
> +
> +		ret = sysfs_create_file(&idev->attr_dir, &uio_attr[i]->attr);
> +		if (ret)
> +			break;
> +	}
> +	if (ret) {
> +		while (--i >= 0)
> +			sysfs_remove_file(&idev->attr_dir, &uio_attr[i]->attr);
> +	}
> +	return ret;
> +}

Ick, you just raced with userspace and blew up any tool that was wanting
to watch for your new kobject to be created with the attributes attached
to it :(

Sending code that doesn't work is fine, for review if you have questions
about things, but please, mark it with a big "I HAVEN'T TESTED THIS"
statement somewhere.

greg k-h

^ permalink raw reply

* Re: [PATCH net v5 1/5] bnx2x: protect different statistics flows
From: Neal Cardwell @ 2013-08-13 17:54 UTC (permalink / raw)
  To: Dmitry Kravkov; +Cc: David Miller, Netdev, eilong, Ariel Elior
In-Reply-To: <1376349903-3461-2-git-send-email-dmitry@broadcom.com>

On Mon, Aug 12, 2013 at 7:24 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> Add locking to protect different statistics flows from
> running simultaneously.
> This in order to serialize statistics requests sent to FW,
> otherwise two outstanding queries may cause FW assert.
>
> Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
> Signed-off-by: Ariel Elior <ariele@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x.h       |  2 +
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  |  1 +
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c | 66 ++++++++++++++++++-----
>  3 files changed, 57 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
> index d80e34b..98be67f 100644

Acked-by: Neal Cardwell <ncardwell@google.com>

Thanks!

neal

^ permalink raw reply

* Re: [PATCH 1/7] uio: add module owner to prevent inappropriate module unloading
From: Greg Kroah-Hartman @ 2013-08-13 17:48 UTC (permalink / raw)
  To: Benedikt Spranger
  Cc: netdev, Alexander Frank, Sebastian Andrzej Siewior, Hans J. Koch,
	Holger Dengler
In-Reply-To: <1376384922-8519-3-git-send-email-b.spranger@linutronix.de>

On Tue, Aug 13, 2013 at 11:08:36AM +0200, Benedikt Spranger wrote:
> If an UIO device is created by another driver (MFD for instance) it has to be
> ensured that the MFD driver isn't removed while the UIO is still in use.

Shouldn't if the MFD driver is removed, the UIO device will be cleaned
up and also removed?  You shouldn't need a module reference for this
type of thing.

And why did I only get 2 of the 7 patches here?

greg k-h

^ permalink raw reply

* Re: [PATCH net v5 0/5] bnx2x: fixes
From: Benjamin Poirier @ 2013-08-13 17:44 UTC (permalink / raw)
  To: Dmitry Kravkov
  Cc: Ariel Elior, davem@davemloft.net, netdev@vger.kernel.org,
	Eilon Greenstein
In-Reply-To: <504C9EFCA2D0054393414C9CB605C37F20D708BF@SJEXCHMB06.corp.ad.broadcom.com>

On 2013/08/13 14:16, Dmitry Kravkov wrote:
[...]
> > 
> > Let me rephrase my question:
> > 
> > I'm confused. I thought that "[PATCH net v4 1/6] bnx2x: properly initialize
> > statistic counters" was meant to fix a race condition at driver startup which
> > causes a second statistics query to be sent before the first one completes,
> > resulting in a firmware assert and a stuck chip. Am I mistaken and there is no
> > such race condition, or is it addressed by the other patches in this series?
> > 
> There is such condition, but it's not caused by wrongly initialized counted -
> the initialization done in a correct way. Patch "[PATCH net v5 1/5] bnx2x: protect
> different statistics flows"  - prevents from two outstanding queries to be sent
> simultaneously.

Thank you for the clarification.

^ permalink raw reply

* Re: tun always return NETDEV_TX_OK, why?
From: Stephen Hemminger @ 2013-08-13 17:33 UTC (permalink / raw)
  To: Yannick Koehler; +Cc: netdev
In-Reply-To: <CAJ4BwwEbBQQ=S5pd=8rHAxnOm-P7yQPv9q2D7c1D9cVh5ArAPQ@mail.gmail.com>

On Tue, 13 Aug 2013 10:39:02 -0400
Yannick Koehler <yannick@koehler.name> wrote:

> Hello,
> 
>   I hit a problem recently with the tun interface, it looks like when
> this interface reach its txqueuelen it will then drop the packet and
> return NETDEV_TX_OK unconditionally.
> 
>  That, from my little understanding of the netdev framework, appears
> to be wrong and will simply eat up any pending buffer and discard them
> until the queue frees itself.  That seems to be against the flow
> control design in the tx queue system of the kernel.
> 
>   So, is this a bug or a misunderstanding?  Would it be ok for tun to
> return NETDEV_TX_BUSY when the txqueuelen is reach and call
> netif_stop_queue() so that the layer above stop sending frame to this
> interface until it can cope it's current queue content?
> 

NETDEV_TX_BUSY won't work well for this because it basically
causes kernel to spin waiting for the transmit queue.
A better way is to figure out how to do proper flow control
with start/stop queue.

^ permalink raw reply

* Re: question about drivers/net/ethernet/jme.{c,h}
From: Joe Perches @ 2013-08-13 17:26 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cooldavid, netdev
In-Reply-To: <alpine.DEB.2.02.1308131831250.2263@hadrien>

On Tue, 2013-08-13 at 18:33 +0200, Julia Lawall wrote:
> I wonder what is the point of the macro NETIF_NAPI_SET?

Today it seems pretty clear it's useless indirection.

It might have been useful sometime before the jme.c
file was first submitted, but now it should be removed.

In the jme.h file on JMicron's website, the driver
still supports linux versions less than 2.6.23 and
uses a form like this:

ftp://driver.jmicron.com.tw/Ethernet/Linux/jmebp-1.0.8.5.tar.bz

#if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,23)
#define NETIF_NAPI_SET(dev, napis, pollfn, q) \
	dev->poll = pollfn; \
	dev->weight = q;
#else
#define NETIF_NAPI_SET(dev, napis, pollfn, q) \
	netif_napi_add(dev, napis, pollfn, q);

^ permalink raw reply

* Re: [PATCH 2/3 v2] ipv6: rename ADBG() to pr_xxx()
From: Joe Perches @ 2013-08-13 16:48 UTC (permalink / raw)
  To: Ding Tianhong
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Netdev, Patrick McHardy
In-Reply-To: <5209E6E8.1030702@huawei.com>

On Tue, 2013-08-13 at 15:57 +0800, Ding Tianhong wrote:
> According to Joe Perches opinions, the ADBG() was tedious and it is
> better to remove the KERN_<LEVEL>s and use the pr_xxx() instead of ADBG().

No, that's not really what I wrote.

I suggested it might be better to remove the KERN_<LEVELS>
from these ADBG uses and use pr_debug.

As is, you've changed logging messages which today
always compile away to nothing to now emit what I
think are pretty unnecessary messages.

^ permalink raw reply

* [RFC] phy: micrel: Convert micrel PHY driver to use OF
From: dinguyen @ 2013-08-13 16:42 UTC (permalink / raw)
  To: dinh.linux
  Cc: Dinh Nguyen, netdev, Richard Cochran, Linus Walleij, Felipe Balbi,
	David S. Miller, Giuseppe Cavallaro, Olof Johansson, Rob Herring

From: Dinh Nguyen <dinguyen@altera.com>

Convert the Micrel PHY driver to use OF. This initial patch is only
the beginning of an idea to convert the PHY driver to device tree.

Signed-of-by: Dinh Nguyen <dinguyen@altera.com>
Cc: netdev@vger.kernel.org
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Felipe Balbi <balbi@ti.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Rob Herring <rob.herring@calxeda.com>

---
Hello,

I would like to solicit comments on the need to convert the ethernet PHY
drivers to use OF/device trees? For the platform that I'm interested in,
SOCFPGA, it is using the stmicro ethernet driver. It has a Micrel PHY
on the board. The only way that I know of how to change the skew settings
for the phy is through a board level initialization.

One of the ARM maintainers suggested that perhaps refactoring the ethernet
driver to use device tree would be nice. But that would not help me with
configuring the PHY settings.

So a little investigation led me to believe that refactoring the /net/phy
drivers into a device tree implementation would help greatly. I was thinking
it could be done like the pinctrl or some of the usb/phy driver.

Since I am only familiar with the ARM SoC space, I want to make sure that
this idea is right approach. I can start with the micrel PHY driver
first, as that is the only HW I have access to.

Thanks for any comments,

Dinh
---
 drivers/net/phy/micrel.c |  107 ++++++++++++++++------------------------------
 1 file changed, 37 insertions(+), 70 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 2510435..6ee1e88 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -174,6 +174,15 @@ static int ksz8873mll_config_aneg(struct phy_device *phydev)
 	return 0;
 }
 
+static const struct phy_driver ksphy_driver_data[] = {
+{
+	.config_init    = kszphy_config_init,
+	.config_aneg    = genphy_config_aneg,
+	.read_status    = genphy_read_status,
+	.ack_interrupt  = kszphy_ack_interrupt,
+	.config_intr    = ks_config_intr,
+};
+
 static struct phy_driver ksphy_driver[] = {
 {
 	.phy_id		= PHY_ID_KS8737,
@@ -181,12 +190,6 @@ static struct phy_driver ksphy_driver[] = {
 	.name		= "Micrel KS8737",
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= ks8737_config_intr,
-	.driver		= { .owner = THIS_MODULE,},
 }, {
 	.phy_id		= PHY_ID_KSZ8021,
 	.phy_id_mask	= 0x00ffffff,
@@ -194,12 +197,6 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
 			   SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= ksz8021_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= kszphy_config_intr,
-	.driver		= { .owner = THIS_MODULE,},
 }, {
 	.phy_id		= PHY_ID_KSZ8031,
 	.phy_id_mask	= 0x00ffffff,
@@ -207,12 +204,6 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
 			   SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= ksz8021_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= kszphy_config_intr,
-	.driver		= { .owner = THIS_MODULE,},
 }, {
 	.phy_id		= PHY_ID_KSZ8041,
 	.phy_id_mask	= 0x00fffff0,
@@ -220,12 +211,6 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
 				| SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= kszphy_config_intr,
-	.driver		= { .owner = THIS_MODULE,},
 }, {
 	.phy_id		= PHY_ID_KSZ8051,
 	.phy_id_mask	= 0x00fffff0,
@@ -233,60 +218,30 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
 				| SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= ks8051_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= kszphy_config_intr,
-	.driver		= { .owner = THIS_MODULE,},
 }, {
 	.phy_id		= PHY_ID_KSZ8001,
 	.name		= "Micrel KSZ8001 or KS8721",
 	.phy_id_mask	= 0x00ffffff,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= kszphy_config_intr,
-	.driver		= { .owner = THIS_MODULE,},
 }, {
 	.phy_id		= PHY_ID_KSZ8081,
 	.name		= "Micrel KSZ8081 or KSZ8091",
 	.phy_id_mask	= 0x00fffff0,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= kszphy_config_intr,
-	.driver		= { .owner = THIS_MODULE,},
 }, {
 	.phy_id		= PHY_ID_KSZ8061,
 	.name		= "Micrel KSZ8061",
 	.phy_id_mask	= 0x00fffff0,
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= kszphy_config_intr,
-	.driver		= { .owner = THIS_MODULE,},
 }, {
 	.phy_id		= PHY_ID_KSZ9021,
 	.phy_id_mask	= 0x000ffffe,
 	.name		= "Micrel KSZ9021 Gigabit PHY",
 	.features	= (PHY_GBIT_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= ksz9021_config_intr,
-	.driver		= { .owner = THIS_MODULE, },
 }, {
 	.phy_id		= PHY_ID_KSZ9031,
 	.phy_id_mask	= 0x00fffff0,
@@ -294,32 +249,18 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= (PHY_GBIT_FEATURES | SUPPORTED_Pause
 				| SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.ack_interrupt	= kszphy_ack_interrupt,
-	.config_intr	= ksz9021_config_intr,
-	.driver		= { .owner = THIS_MODULE, },
 }, {
 	.phy_id		= PHY_ID_KSZ8873MLL,
 	.phy_id_mask	= 0x00fffff0,
 	.name		= "Micrel KSZ8873MLL Switch",
 	.features	= (SUPPORTED_Pause | SUPPORTED_Asym_Pause),
 	.flags		= PHY_HAS_MAGICANEG,
-	.config_init	= kszphy_config_init,
-	.config_aneg	= ksz8873mll_config_aneg,
-	.read_status	= ksz8873mll_read_status,
-	.driver		= { .owner = THIS_MODULE, },
 }, {
 	.phy_id		= PHY_ID_KSZ886X,
 	.phy_id_mask	= 0x00fffff0,
 	.name		= "Micrel KSZ886X Switch",
 	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
-	.config_init	= kszphy_config_init,
-	.config_aneg	= genphy_config_aneg,
-	.read_status	= genphy_read_status,
-	.driver		= { .owner = THIS_MODULE, },
 } };
 
 static int __init ksphy_init(void)
@@ -334,8 +275,34 @@ static void __exit ksphy_exit(void)
 		ARRAY_SIZE(ksphy_driver));
 }
 
-module_init(ksphy_init);
-module_exit(ksphy_exit);
+static const struct of_device_id phy_micrel_match[] = {
+	{ .compatible = "micrel,ks8721", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ks8737", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8001", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8021", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8031", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8041", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8051", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8061", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8081", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8091", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8021-gigabit", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8031-gigabit", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz8873mll-switch", .data = &ksphy_driver_data },
+	{ .compatible = "micrel,ksz886x-switch", .data = &ksphy_driver_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, phy_micrel_match);
+
+static struct platform_driver phy_micrel_driver = {
+	.probe          = ksphy_init,
+	.remove         = ksphy_exit,
+	.driver         = {
+		.name           = "phy_micrel",
+		.of_match_table = of_match_ptr(phy_micrel_match),
+	},
+};
+module_platform_driver(phy_micrel_driver);
 
 MODULE_DESCRIPTION("Micrel PHY driver");
 MODULE_AUTHOR("David J. Choi");
-- 
1.7.9.5

^ permalink raw reply related

* question about drivers/net/ethernet/jme.{c,h}
From: Julia Lawall @ 2013-08-13 16:33 UTC (permalink / raw)
  To: cooldavid, netdev

I wonder what is the point of the macro NETIF_NAPI_SET?  It has only one
definition, and it just transfers its arguments directly to the more
common function netif_napi_add.  The fact that the definition of the macro
contains a semicolon also makes the use of NETIF_NAPI_SET not look like a
normal function call.

thanks,
julia

^ permalink raw reply

* [PATCH net-next v2 3/4] ipip: add x-netns support
From: Nicolas Dichtel @ 2013-08-13 15:51 UTC (permalink / raw)
  To: netdev
  Cc: davem, ebiederm, bcrl, ravi.mlists, bhutchings, eric.dumazet,
	Nicolas Dichtel
In-Reply-To: <1376409072-6414-1-git-send-email-nicolas.dichtel@6wind.com>

This patch allows to switch the netns when packet is encapsulated or
decapsulated. In other word, the encapsulated packet is received in a netns,
where the lookup is done to find the tunnel. Once the tunnel is found, the
packet is decapsulated and injecting into the corresponding interface which
stands to another netns.

When one of the two netns is removed, the tunnel is destroyed.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/net/ip_tunnels.h |  2 +-
 net/ipv4/ip_gre.c        |  4 ++--
 net/ipv4/ip_tunnel.c     | 43 ++++++++++++++++++++++++++++---------------
 net/ipv4/ip_vti.c        |  2 +-
 net/ipv4/ipip.c          |  3 +--
 5 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index c6acd9f8f877..5a76f2bef822 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -102,7 +102,7 @@ void  ip_tunnel_dellink(struct net_device *dev, struct list_head *head);
 int ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
 		       struct rtnl_link_ops *ops, char *devname);
 
-void ip_tunnel_delete_net(struct ip_tunnel_net *itn);
+void ip_tunnel_delete_net(struct ip_tunnel_net *itn, struct rtnl_link_ops *ops);
 
 void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 		    const struct iphdr *tnl_params, const u8 protocol);
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 1f6eab66f7ce..bc3a76521deb 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -534,7 +534,7 @@ static int __net_init ipgre_init_net(struct net *net)
 static void __net_exit ipgre_exit_net(struct net *net)
 {
 	struct ip_tunnel_net *itn = net_generic(net, ipgre_net_id);
-	ip_tunnel_delete_net(itn);
+	ip_tunnel_delete_net(itn, &ipgre_link_ops);
 }
 
 static struct pernet_operations ipgre_net_ops = {
@@ -767,7 +767,7 @@ static int __net_init ipgre_tap_init_net(struct net *net)
 static void __net_exit ipgre_tap_exit_net(struct net *net)
 {
 	struct ip_tunnel_net *itn = net_generic(net, gre_tap_net_id);
-	ip_tunnel_delete_net(itn);
+	ip_tunnel_delete_net(itn, &ipgre_tap_ops);
 }
 
 static struct pernet_operations ipgre_tap_net_ops = {
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index a351a003ee6b..a4d9126c7b51 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -350,7 +350,7 @@ static int ip_tunnel_bind_dev(struct net_device *dev)
 		struct flowi4 fl4;
 		struct rtable *rt;
 
-		rt = ip_route_output_tunnel(dev_net(dev), &fl4,
+		rt = ip_route_output_tunnel(tunnel->net, &fl4,
 					    tunnel->parms.iph.protocol,
 					    iph->daddr, iph->saddr,
 					    tunnel->parms.o_key,
@@ -365,7 +365,7 @@ static int ip_tunnel_bind_dev(struct net_device *dev)
 	}
 
 	if (!tdev && tunnel->parms.link)
-		tdev = __dev_get_by_index(dev_net(dev), tunnel->parms.link);
+		tdev = __dev_get_by_index(tunnel->net, tunnel->parms.link);
 
 	if (tdev) {
 		hlen = tdev->hard_header_len + tdev->needed_headroom;
@@ -654,7 +654,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 		}
 	}
 
-	err = iptunnel_xmit(dev_net(dev), rt, skb,
+	err = iptunnel_xmit(tunnel->net, rt, skb,
 			    fl4.saddr, fl4.daddr, protocol,
 			    ip_tunnel_ecn_encap(tos, inner_iph, skb), ttl, df);
 	iptunnel_xmit_stats(err, &dev->stats, dev->tstats);
@@ -821,11 +821,10 @@ static void ip_tunnel_dev_free(struct net_device *dev)
 
 void ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
 {
-	struct net *net = dev_net(dev);
 	struct ip_tunnel *tunnel = netdev_priv(dev);
 	struct ip_tunnel_net *itn;
 
-	itn = net_generic(net, tunnel->ip_tnl_net_id);
+	itn = net_generic(tunnel->net, tunnel->ip_tnl_net_id);
 
 	if (itn->fb_tunnel_dev != dev) {
 		ip_tunnel_del(netdev_priv(dev));
@@ -855,6 +854,10 @@ int ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
 
 	rtnl_lock();
 	itn->fb_tunnel_dev = __ip_tunnel_create(net, ops, &parms);
+	/* FB netdevice is special: we have one, and only one per netns.
+	 * Allowing to move it to another netns is clearly unsafe.
+	 */
+	itn->fb_tunnel_dev->features |= NETIF_F_NETNS_LOCAL;
 	rtnl_unlock();
 
 	if (IS_ERR(itn->fb_tunnel_dev))
@@ -864,28 +867,39 @@ int ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_init_net);
 
-static void ip_tunnel_destroy(struct ip_tunnel_net *itn, struct list_head *head)
+static void ip_tunnel_destroy(struct ip_tunnel_net *itn, struct list_head *head,
+			      struct rtnl_link_ops *ops)
 {
+	struct net *net = dev_net(itn->fb_tunnel_dev);
+	struct net_device *dev, *aux;
 	int h;
 
+	for_each_netdev_safe(net, dev, aux)
+		if (dev->rtnl_link_ops == ops)
+			unregister_netdevice_queue(dev, head);
+
 	for (h = 0; h < IP_TNL_HASH_SIZE; h++) {
 		struct ip_tunnel *t;
 		struct hlist_node *n;
 		struct hlist_head *thead = &itn->tunnels[h];
 
 		hlist_for_each_entry_safe(t, n, thead, hash_node)
-			unregister_netdevice_queue(t->dev, head);
+			/* If dev is in the same netns, it has already
+			 * been added to the list by the previous loop.
+			 */
+			if (!net_eq(dev_net(t->dev), net))
+				unregister_netdevice_queue(t->dev, head);
 	}
 	if (itn->fb_tunnel_dev)
 		unregister_netdevice_queue(itn->fb_tunnel_dev, head);
 }
 
-void ip_tunnel_delete_net(struct ip_tunnel_net *itn)
+void ip_tunnel_delete_net(struct ip_tunnel_net *itn, struct rtnl_link_ops *ops)
 {
 	LIST_HEAD(list);
 
 	rtnl_lock();
-	ip_tunnel_destroy(itn, &list);
+	ip_tunnel_destroy(itn, &list, ops);
 	unregister_netdevice_many(&list);
 	rtnl_unlock();
 }
@@ -929,23 +943,21 @@ EXPORT_SYMBOL_GPL(ip_tunnel_newlink);
 int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
 			 struct ip_tunnel_parm *p)
 {
-	struct ip_tunnel *t, *nt;
-	struct net *net = dev_net(dev);
+	struct ip_tunnel *t;
 	struct ip_tunnel *tunnel = netdev_priv(dev);
+	struct net *net = tunnel->net;
 	struct ip_tunnel_net *itn = net_generic(net, tunnel->ip_tnl_net_id);
 
 	if (dev == itn->fb_tunnel_dev)
 		return -EINVAL;
 
-	nt = netdev_priv(dev);
-
 	t = ip_tunnel_find(itn, p, dev->type);
 
 	if (t) {
 		if (t->dev != dev)
 			return -EEXIST;
 	} else {
-		t = nt;
+		t = tunnel;
 
 		if (dev->type != ARPHRD_ETHER) {
 			unsigned int nflags = 0;
@@ -984,6 +996,7 @@ int ip_tunnel_init(struct net_device *dev)
 	}
 
 	tunnel->dev = dev;
+	tunnel->net = dev_net(dev);
 	strcpy(tunnel->parms.name, dev->name);
 	iph->version		= 4;
 	iph->ihl		= 5;
@@ -994,8 +1007,8 @@ EXPORT_SYMBOL_GPL(ip_tunnel_init);
 
 void ip_tunnel_uninit(struct net_device *dev)
 {
-	struct net *net = dev_net(dev);
 	struct ip_tunnel *tunnel = netdev_priv(dev);
+	struct net *net = tunnel->net;
 	struct ip_tunnel_net *itn;
 
 	itn = net_generic(net, tunnel->ip_tnl_net_id);
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 79b263da4168..e805e7b3030e 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -318,7 +318,7 @@ static int __net_init vti_init_net(struct net *net)
 static void __net_exit vti_exit_net(struct net *net)
 {
 	struct ip_tunnel_net *itn = net_generic(net, vti_net_id);
-	ip_tunnel_delete_net(itn);
+	ip_tunnel_delete_net(itn, &vti_link_ops);
 }
 
 static struct pernet_operations vti_net_ops = {
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 51fc2a1dcdd3..87bd2952c733 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -286,7 +286,6 @@ static void ipip_tunnel_setup(struct net_device *dev)
 	dev->flags		= IFF_NOARP;
 	dev->iflink		= 0;
 	dev->addr_len		= 4;
-	dev->features		|= NETIF_F_NETNS_LOCAL;
 	dev->features		|= NETIF_F_LLTX;
 	dev->priv_flags		&= ~IFF_XMIT_DST_RELEASE;
 
@@ -437,7 +436,7 @@ static int __net_init ipip_init_net(struct net *net)
 static void __net_exit ipip_exit_net(struct net *net)
 {
 	struct ip_tunnel_net *itn = net_generic(net, ipip_net_id);
-	ip_tunnel_delete_net(itn);
+	ip_tunnel_delete_net(itn, &ipip_link_ops);
 }
 
 static struct pernet_operations ipip_net_ops = {
-- 
1.8.2.1

^ permalink raw reply related


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