* [PATCH 2/7] phy: miphy365x: Pass sysconfig register offsets via syscfg dt property.
From: Peter Griffin @ 2014-11-19 8:27 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
maxime.coquelin, patrice.chotard, peppe.cavallaro, kishon, arnd
Cc: peter.griffin, netdev, lee.jones, alexandre.torgue, devicetree
In-Reply-To: <1416385632-5832-1-git-send-email-peter.griffin@linaro.org>
Based on Arnds review comments here https://lkml.org/lkml/2014/11/13/161, update
the miphy365 phy driver to access sysconfig register offsets via syscfg dt property.
This is because the reg property should not be mixing address spaces like it does
currently for miphy365. This change then also aligns us to how other platforms such
as keystone and bcm7445 pass there syscon offsets via DT.
This patch breaks DT compatibility, but this platform is considered WIP, and is only
used by a few developers who are upstreaming support for it. This change has been done
as a single atomic commit to ensure it is bisectable.
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
.../devicetree/bindings/phy/phy-miphy365x.txt | 15 +++++------
arch/arm/boot/dts/stih416.dtsi | 10 ++++----
drivers/phy/phy-miphy365x.c | 29 ++++++++--------------
3 files changed, 23 insertions(+), 31 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
index 42c8808..9802d5d 100644
--- a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
+++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
@@ -6,8 +6,10 @@ for SATA and PCIe.
Required properties (controller (parent) node):
- compatible : Should be "st,miphy365x-phy"
-- st,syscfg : Should be a phandle of the system configuration register group
- which contain the SATA, PCIe mode setting bits
+- st,syscfg : Phandle / integer array property. Phandle of sysconfig group
+ containing the miphy registers and integer array should contain
+ an entry for each port sub-node, specifying the control
+ register offset inside the sysconfig group.
Required nodes : A sub-node is required for each channel the controller
provides. Address range information including the usual
@@ -26,7 +28,6 @@ Required properties (port (child) node):
registers filled in "reg":
- sata: For SATA devices
- pcie: For PCIe devices
- - syscfg: To specify the syscfg based config register
Optional properties (port (child) node):
- st,sata-gen : Generation of locally attached SATA IP. Expected values
@@ -39,20 +40,20 @@ Example:
miphy365x_phy: miphy365x@fe382000 {
compatible = "st,miphy365x-phy";
- st,syscfg = <&syscfg_rear>;
+ st,syscfg = <&syscfg_rear 0x824 0x828>;
#address-cells = <1>;
#size-cells = <1>;
ranges;
phy_port0: port@fe382000 {
- reg = <0xfe382000 0x100>, <0xfe394000 0x100>, <0x824 0x4>;
- reg-names = "sata", "pcie", "syscfg";
+ reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
+ reg-names = "sata", "pcie";
#phy-cells = <1>;
st,sata-gen = <3>;
};
phy_port1: port@fe38a000 {
- reg = <0xfe38a000 0x100>, <0xfe804000 0x100>, <0x828 0x4>;;
+ reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;;
reg-names = "sata", "pcie", "syscfg";
#phy-cells = <1>;
st,pcie-tx-pol-inv;
diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index fad9073..6fbde39 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -283,21 +283,21 @@
miphy365x_phy: phy@fe382000 {
compatible = "st,miphy365x-phy";
- st,syscfg = <&syscfg_rear>;
+ st,syscfg = <&syscfg_rear 0x824 0x828>;
#address-cells = <1>;
#size-cells = <1>;
ranges;
phy_port0: port@fe382000 {
#phy-cells = <1>;
- reg = <0xfe382000 0x100>, <0xfe394000 0x100>, <0x824 0x4>;
- reg-names = "sata", "pcie", "syscfg";
+ reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
+ reg-names = "sata", "pcie";
};
phy_port1: port@fe38a000 {
#phy-cells = <1>;
- reg = <0xfe38a000 0x100>, <0xfe804000 0x100>, <0x828 0x4>;
- reg-names = "sata", "pcie", "syscfg";
+ reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;
+ reg-names = "sata", "pcie";
};
};
diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
index 801afaf..7308afe 100644
--- a/drivers/phy/phy-miphy365x.c
+++ b/drivers/phy/phy-miphy365x.c
@@ -141,7 +141,7 @@ struct miphy365x_phy {
bool pcie_tx_pol_inv;
bool sata_tx_pol_inv;
u32 sata_gen;
- u64 ctrlreg;
+ u32 ctrlreg;
u8 type;
};
@@ -179,7 +179,7 @@ static int miphy365x_set_path(struct miphy365x_phy *miphy_phy,
bool sata = (miphy_phy->type == MIPHY_TYPE_SATA);
return regmap_update_bits(miphy_dev->regmap,
- (unsigned int)miphy_phy->ctrlreg,
+ miphy_phy->ctrlreg,
SYSCFG_SELECT_SATA_MASK,
sata << SYSCFG_SELECT_SATA_POS);
}
@@ -445,7 +445,6 @@ int miphy365x_get_addr(struct device *dev, struct miphy365x_phy *miphy_phy,
{
struct device_node *phynode = miphy_phy->phy->dev.of_node;
const char *name;
- const __be32 *taddr;
int type = miphy_phy->type;
int ret;
@@ -455,22 +454,6 @@ int miphy365x_get_addr(struct device *dev, struct miphy365x_phy *miphy_phy,
return ret;
}
- if (!strncmp(name, "syscfg", 6)) {
- taddr = of_get_address(phynode, index, NULL, NULL);
- if (!taddr) {
- dev_err(dev, "failed to fetch syscfg address\n");
- return -EINVAL;
- }
-
- miphy_phy->ctrlreg = of_translate_address(phynode, taddr);
- if (miphy_phy->ctrlreg == OF_BAD_ADDR) {
- dev_err(dev, "failed to translate syscfg address\n");
- return -EINVAL;
- }
-
- return 0;
- }
-
if (!((!strncmp(name, "sata", 4) && type == MIPHY_TYPE_SATA) ||
(!strncmp(name, "pcie", 4) && type == MIPHY_TYPE_PCIE)))
return 0;
@@ -606,7 +589,15 @@ static int miphy365x_probe(struct platform_device *pdev)
return ret;
phy_set_drvdata(phy, miphy_dev->phys[port]);
+
port++;
+ /*sysconfig offsets are not indexed from zero */
+ ret = of_property_read_u32_index(np, "st,syscfg", port,
+ &miphy_phy->ctrlreg);
+ if (ret) {
+ dev_err(&pdev->dev, "No sysconfig offset found\n");
+ return ret;
+ }
}
provider = devm_of_phy_provider_register(&pdev->dev, miphy365x_xlate);
--
1.9.1
^ permalink raw reply related
* [PATCH 1/7] phy: phy-stih407-usb: Pass sysconfig register offsets via syscfg property.
From: Peter Griffin @ 2014-11-19 8:27 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
maxime.coquelin, patrice.chotard, peppe.cavallaro, kishon, arnd
Cc: peter.griffin, lee.jones, netdev, devicetree, alexandre.torgue
In-Reply-To: <1416385632-5832-1-git-send-email-peter.griffin@linaro.org>
Based on Arnds review comments here https://lkml.org/lkml/2014/11/13/161, update
the phy driver to not use the reg property to access the sysconfig register offsets.
This is because other phy's (miphy28, miphy365) have a combination of memory mapped
registers and sysconfig control regs, and we shouldn't be mixing address spaces
in the reg property. In addition we would ideally like the sysconfig offsets to be
passed via DT in a uniform way. This new method will also allow us to support devices
which have sysconfig registers in different banks more easily and it is also analagous
to how keystone and bcm7745 platforms pass there syscon offsets in DT.
This breaks DT compatibility, but this platform is considered WIP, and is only
used by a few developers who are upstreaming support for it.
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
.../devicetree/bindings/phy/phy-stih407-usb.txt | 10 ++-------
drivers/phy/phy-stih407-usb.c | 25 ++++++++++++----------
2 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/phy-stih407-usb.txt b/Documentation/devicetree/bindings/phy/phy-stih407-usb.txt
index 1ef8228..de6a706 100644
--- a/Documentation/devicetree/bindings/phy/phy-stih407-usb.txt
+++ b/Documentation/devicetree/bindings/phy/phy-stih407-usb.txt
@@ -5,10 +5,7 @@ host controllers (when controlling usb2/1.1 devices) available on STiH407 SoC fa
Required properties:
- compatible : should be "st,stih407-usb2-phy"
-- reg : contain the offset and length of the system configuration registers
- used as glue logic to control & parameter phy
-- reg-names : the names of the system configuration registers in "reg", should be "param" and "reg"
-- st,syscfg : sysconfig register to manage phy parameter at driver level
+- st,syscfg : phandle of sysconfig bank plus integer array containing phyparam and phyctrl register offsets
- resets : list of phandle and reset specifier pairs. There should be two entries, one
for the whole phy and one for the port
- reset-names : list of reset signal names. Should be "global" and "port"
@@ -19,11 +16,8 @@ Example:
usb2_picophy0: usbpicophy@f8 {
compatible = "st,stih407-usb2-phy";
- reg = <0xf8 0x04>, /* syscfg 5062 */
- <0xf4 0x04>; /* syscfg 5061 */
- reg-names = "param", "ctrl";
#phy-cells = <0>;
- st,syscfg = <&syscfg_core>;
+ st,syscfg = <&syscfg_core 0x100 0xf4>;
resets = <&softreset STIH407_PICOPHY_SOFTRESET>,
<&picophyreset STIH407_PICOPHY0_RESET>;
reset-names = "global", "port";
diff --git a/drivers/phy/phy-stih407-usb.c b/drivers/phy/phy-stih407-usb.c
index 42428d4..24d5e2a 100644
--- a/drivers/phy/phy-stih407-usb.c
+++ b/drivers/phy/phy-stih407-usb.c
@@ -22,6 +22,9 @@
#include <linux/mfd/syscon.h>
#include <linux/phy/phy.h>
+#define PHYPARAM_REG 1
+#define PHYCTRL_REG 2
+
/* Default PHY_SEL and REFCLKSEL configuration */
#define STIH407_USB_PICOPHY_CTRL_PORT_CONF 0x6
#define STIH407_USB_PICOPHY_CTRL_PORT_MASK 0x1f
@@ -93,7 +96,7 @@ static int stih407_usb2_picophy_probe(struct platform_device *pdev)
struct device_node *np = dev->of_node;
struct phy_provider *phy_provider;
struct phy *phy;
- struct resource *res;
+ int ret;
phy_dev = devm_kzalloc(dev, sizeof(*phy_dev), GFP_KERNEL);
if (!phy_dev)
@@ -123,19 +126,19 @@ static int stih407_usb2_picophy_probe(struct platform_device *pdev)
return PTR_ERR(phy_dev->regmap);
}
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl");
- if (!res) {
- dev_err(dev, "No ctrl reg found\n");
- return -ENXIO;
+ ret = of_property_read_u32_index(np, "st,syscfg", PHYPARAM_REG,
+ &phy_dev->param);
+ if (ret) {
+ dev_err(dev, "can't get phyparam offset (%d)\n", ret);
+ return ret;
}
- phy_dev->ctrl = res->start;
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "param");
- if (!res) {
- dev_err(dev, "No param reg found\n");
- return -ENXIO;
+ ret = of_property_read_u32_index(np, "st,syscfg", PHYCTRL_REG,
+ &phy_dev->ctrl);
+ if (ret) {
+ dev_err(dev, "can't get phyctrl offset (%d)\n", ret);
+ return ret;
}
- phy_dev->param = res->start;
phy = devm_phy_create(dev, NULL, &stih407_usb2_picophy_data, NULL);
if (IS_ERR(phy)) {
--
1.9.1
^ permalink raw reply related
* [PATCH 0/7] Fix sti drivers whcih mix reg address spaces
From: Peter Griffin @ 2014-11-19 8:27 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
srinivas.kandagatla-Re5JQEeQqe8AvxtiuMwx3w,
maxime.coquelin-qxv4g6HH51o, patrice.chotard-qxv4g6HH51o,
peppe.cavallaro-qxv4g6HH51o, kishon-l0cyMroinI0, arnd-r2nGTMty4D4
Cc: peter.griffin-QSEj5FYQhm4dnm+yROfE0A,
lee.jones-QSEj5FYQhm4dnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, alexandre.torgue-qxv4g6HH51o
Hi,
Following on from Arnds comments about the picophy driver here
https://lkml.org/lkml/2014/11/13/161, this series fixes the
remaining upstreamed drivers for STI, which are mixing address spaces
in the reg property. We do this in a way similar to the keystone
and bcm7445 platforms, by having sysconfig phandle/ offset pair (
where only one register is required). Or phandle / integer array
where multiple offsets in the same bank are needed).
This series breaks DT compatability! But the platform support
is WIP and only being used by the few developers who are upstreaming
support for it. I've made each change to the driver / dt doc / dt
file as a single atomic commit so the kernel will remain bisectable.
This series then also enables the picophy driver, and adds back in
the ehci/ohci dt nodes for stih410 which make use of the picophy.
regards,
Peter.
Peter Griffin (7):
phy: phy-stih407-usb: Pass sysconfig register offsets via syscfg
property.
phy: miphy365x: Pass sysconfig register offsets via syscfg dt
property.
ARM: STi: DT: STiH407: Add usb2 picophy dt nodes
ARM: STi: DT: STiH410: Add usb2 picophy dt nodes
ARM: STi: DT: STiH410: Add DT nodes for the ehci and ohci usb
controllers.
ARM: multi_v7_defconfig: Enable stih407 usb picophy
stmmac: dwmac-sti: Pass sysconfig register offset via syscon dt
property.
.../devicetree/bindings/net/sti-dwmac.txt | 14 ++---
.../devicetree/bindings/phy/phy-miphy365x.txt | 15 ++---
.../devicetree/bindings/phy/phy-stih407-usb.txt | 10 +---
arch/arm/boot/dts/stih407-family.dtsi | 9 +++
arch/arm/boot/dts/stih410.dtsi | 70 ++++++++++++++++++++++
arch/arm/boot/dts/stih415.dtsi | 12 ++--
arch/arm/boot/dts/stih416.dtsi | 22 +++----
arch/arm/configs/multi_v7_defconfig | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c | 13 ++--
drivers/phy/phy-miphy365x.c | 29 ++++-----
drivers/phy/phy-stih407-usb.c | 25 ++++----
11 files changed, 143 insertions(+), 77 deletions(-)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [BNX2] A Netdev Watchdog with kernel stable 3.4
From: Rui Xiang @ 2014-11-19 8:21 UTC (permalink / raw)
To: Michael Chan, sony.chacko; +Cc: netdev
In-Reply-To: <1416380319.6396.15.camel@LTIRV-MCHAN1.corp.ad.broadcom.com>
Thank you for your comments and attention, Michael.
And welcome Sony's advise. :)
On 2014/11/19 14:58, Michael Chan wrote:
> Copying the current maintainer Sony. The PCI command register looks
> strange. Please see below.
>
> On Wed, 2014-11-19 at 14:28 +0800, Rui Xiang wrote:
>> ping...
>>
>> On 2014/11/17 20:42, Rui Xiang wrote:
>>> Hi Michael,
>>>
>>> On a system that was running stable 3.4.87, I got the below stack.
>>> That was a NETDEV WATCHDOG. And we could also see watchdog timeouts with the
>>> BNX2. (After the stack, an oops occurred while running ifconfig. I think it
>>> would be related to this timeout.)
>>>
>>> Otherwises, the bnx2_dump_state and bnx2_dump_mcp_state have printed the states.
>>> Through these states info, can we got the real situation of NIC1.
>>> Or can we see what resulted the WATCHDOG, a bnx2 device fault or other reasons.
>>>
>>> Thanks.
>>>
>>>
>>> *The stack*:
>>>
>>> WARNING: at /usr/src/packages/BUILD/kernel-default-3.4.87/linux-3.4/net/sched/sch_generic.c:256 dev_watchdog+0x256/0x260()
>>> NETDEV WATCHDOG: NIC1 (bnx2): transmit queue 3 timed out
>>> Modules linked in: smb3_failover(O) smb2(O) smb(O) smb_manager(O) nfs(O) nfs_acl(O) nfsd(O) lockd(O) nal(O) auth_rpcgss(O)
>>> scsi_dh_hp_sw scsi_dh_alua scsi_dh_emc scsi_dh_rdac scsi_dh scsi_mod [last unloaded: ipmi_msghandler]
>>> Pid: 0, comm: swapper/0 Tainted: P W O 3.4.87-default #1
>>> Call Trace:
>>> <IRQ> [<ffffffff8103fcea>] warn_slowpath_common+0x7a/0xb0
>>> [<ffffffff8103fdc1>] warn_slowpath_fmt+0x41/0x50
>>> [<ffffffff81047749>] ? raise_softirq_irqoff+0x9/0x30
>>> [<ffffffff813ae0f6>] dev_watchdog+0x256/0x260
>>> [<ffffffff813adea0>] ? dev_deactivate_queue.constprop.30+0x70/0x70
>>> [<ffffffff8104edc7>] run_timer_softirq+0x147/0x340
>>> [<ffffffff810470d8>] __do_softirq+0xc8/0x1e0
>>> [<ffffffff8109250f>] ? tick_program_event+0x1f/0x30
>>> [<ffffffff81460a6c>] call_softirq+0x1c/0x30
>>> [<ffffffff8100417d>] do_softirq+0x9d/0xd0
>>> [<ffffffff810474a5>] irq_exit+0xb5/0xc0
>>> [<ffffffff81021b49>] smp_apic_timer_interrupt+0x69/0xa0
>>> [<ffffffff8146006f>] apic_timer_interrupt+0x6f/0x80
>>> <EOI> [<ffffffff81457bdd>] ? retint_restore_args+0x13/0x13
>>> [<ffffffff81360149>] ? poll_idle+0x49/0x90
>>> [<ffffffff8136011f>] ? poll_idle+0x1f/0x90
>>> [<ffffffff8135fcc9>] cpuidle_enter+0x19/0x20
>>> [<ffffffff813602f2>] cpuidle_idle_call+0xa2/0x250
>>> [<ffffffff8100b08f>] cpu_idle+0x6f/0xe0
>>> [<ffffffff81915960>] ? rawsock_init+0x12/0x12
>>> [<ffffffff814331c9>] rest_init+0x6d/0x74
>>> [<ffffffff818d3be5>] start_kernel+0x3a2/0x3af
>>> [<ffffffff818d3642>] ? repair_env_string+0x5e/0x5e
>>> [<ffffffff818d332a>] x86_64_start_reservations+0x131/0x135
>>> [<ffffffff818d342e>] x86_64_start_kernel+0x100/0x10f
>>> ---[ end trace 497e24e681e0c02d ]---
>>> bnx2 0000:05:00.1: NIC1: DEBUG: intr_sem[0] PCI_CMD[00100002]
>
> The memory bit in PCI_CMD is set, but the bus master bit is not set.
> DMA won't work if the bus master bit is not set. What was happening
> before the timeout? Was it working fine for a while and it suddenly
> stopped?
>
>From the dmesg, I think it should be stopped suddenly. Because before this timeout, It seems
that nic1 can handle packages naturally.
[ 1882.809865] IPv4: martian source 93.93.255.255 from 93.93.41.1, on dev NIC1
[ 1882.809871] ll header: 00000000: ff ff ff ff ff ff 04 f9 38 85 c7 a6 08 00 ........8.....
After about 10 seconds, the timeout happens. And during the 10 seconds, there is no other exception.
[ 1892.991286] NETDEV WATCHDOG: NIC1 (bnx2): transmit queue 3 timed out
So from your experience, which pathes could result in the bus master bit not set, or an hardware error?
Thanks.
>>> bnx2 0000:05:00.1: NIC1: DEBUG: PCI_PM[19002008] PCI_MISC_CFG[92000088]
>>> bnx2 0000:05:00.1: NIC1: DEBUG: EMAC_TX_STATUS[00000008] EMAC_RX_STATUS[00000000]
>>> bnx2 0000:05:00.1: NIC1: DEBUG: RPM_MGMT_PKT_CTRL[40000088]
>>> bnx2 0000:05:00.1: NIC1: DEBUG: HC_STATS_INTERRUPT_STATUS[01ff0000]
>>> bnx2 0000:05:00.1: NIC1: DEBUG: PBA[00000000]
>>> bnx2 0000:05:00.1: NIC1: <--- start MCP states dump --->
>>> bnx2 0000:05:00.1: NIC1: DEBUG: MCP_STATE_P0[0003e10e] MCP_STATE_P1[0003e10e]
>>> bnx2 0000:05:00.1: NIC1: DEBUG: MCP mode[0000b800] state[80008000] evt_mask[00000500]
>>> bnx2 0000:05:00.1: NIC1: DEBUG: pc[08008f60] pc[0800d21c] instr[00051080]
>>> bnx2 0000:05:00.1: NIC1: DEBUG: shmem states:
>>> bnx2 0000:05:00.1: NIC1: DEBUG: drv_mb[01030003] fw_mb[00000003] link_status[0000006f] drv_pulse_mb[0000073d]
>>> bnx2 0000:05:00.1: NIC1: DEBUG: dev_info_signature[44564907] reset_type[01005254] condition[0003e10e]
>>> bnx2 0000:05:00.1: NIC1: DEBUG: 000003cc: 00000000 00000000 00000000 00000000
>>> bnx2 0000:05:00.1: NIC1: DEBUG: 000003dc: 00000000 00000000 00000000 00000000
>>> bnx2 0000:05:00.1: NIC1: DEBUG: 000003ec: 00000000 00000000 00000000 00000000
>>> bnx2 0000:05:00.1: NIC1: DEBUG: 0x3fc[00000000]
>>> bnx2 0000:05:00.1: NIC1: <--- end MCP states dump --->
>>>
>>
>>
>
>
>
> .
>
^ permalink raw reply
* Re: [ovs-dev] [PATCH net] openvswitch: Fix mask generation for IPv6 labels.
From: Pravin Shelar @ 2014-11-19 8:11 UTC (permalink / raw)
To: Joe Stringer; +Cc: dev@openvswitch.org, netdev, LKML
In-Reply-To: <CAOftzPgYBBFTfNZunf4oGSmECzjOKF0=dnQ9EkTDFsDi+9jfNw@mail.gmail.com>
On Tue, Nov 18, 2014 at 11:25 PM, Joe Stringer <joestringer@nicira.com> wrote:
> On 18 November 2014 22:09, Pravin Shelar <pshelar@nicira.com> wrote:
>>
>> On Tue, Nov 18, 2014 at 10:54 AM, Joe Stringer <joestringer@nicira.com>
>> wrote:
>> > When userspace doesn't provide a mask, OVS datapath generates a fully
>> > unwildcarded mask for the flow. This is done by taking a copy of the
>> > flow key, then iterating across its attributes, setting all values to
>> > 0xff. This works for most attributes, as the length of the netlink
>> > attribute typically matches the length of the value. However, IPv6
>> > labels only use the lower 20 bits of the field. This patch makes a
>> > special case to handle this.
>> >
>> > This fixes the following error seen when installing IPv6 flows without a
>> > mask:
>> >
>> > openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff,
>> > max=fffff)
>> >
>> We should allow exact match mask here rather than generating
>> wildcarded mask. So that ovs can catch invalid ipv6.label.
>
>
> I don't quite follow, I thought this was exact-match? (The existing function
> sets all bits to 1)
>
With 0xffffffff value we can exact match on all ipv6.lable bits.
> In this case, userspace has not specified a mask, but the kernel complains
> about a mask that is too wide (because it generated a mask that's too wide).
> Do you have an alternative fix in mind?
We can avoid the sanity check ipv6.lable for mask key.
^ permalink raw reply
* Re: [patch net-next v3 8/9] net: move vlan pop/push functions into common code
From: Pravin Shelar @ 2014-11-19 8:06 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Jamal Hadi Salim, Tom Herbert, Eric Dumazet,
Willem de Bruijn, Daniel Borkmann, mst, fw, Paul.Durrant,
Thomas Graf, Cong Wang
In-Reply-To: <1416346664-9290-9-git-send-email-jiri@resnulli.us>
On Tue, Nov 18, 2014 at 1:37 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> So it can be used from out of openvswitch code.
> Did couple of cosmetic changes on the way, namely variable naming and
> adding support for 8021AD proto.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>
> v2->v3
> - used previously introduced __vlan_insert_tag helper
> - used skb_push/pull to get skb->data into needed point
> - fixed skb->mac_len computation in skb_vlan_push pointed out by Pravin
> v1->v2:
> - adjusted to fix recent ovs changes
> - included change to use make_writable suggested by Eric
>
> include/linux/skbuff.h | 2 +
> net/core/skbuff.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
> net/openvswitch/actions.c | 88 ++++++-------------------------------------
> 3 files changed, 109 insertions(+), 76 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index e045516..78c299f 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2679,6 +2679,8 @@ unsigned int skb_gso_transport_seglen(const struct sk_buff *skb);
> struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
> struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
> int skb_ensure_writable(struct sk_buff *skb, int write_len);
> +int skb_vlan_pop(struct sk_buff *skb);
> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
>
> struct skb_checksum_ops {
> __wsum (*update)(const void *mem, int len, __wsum wsum);
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index d11bbe0..2f72e62 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4163,6 +4163,101 @@ int skb_ensure_writable(struct sk_buff *skb, int write_len)
> }
> EXPORT_SYMBOL(skb_ensure_writable);
>
> +/* remove VLAN header from packet and update csum accordingly. */
> +static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
> +{
> + struct vlan_hdr *vhdr;
> + unsigned int offset = skb->data - skb_mac_header(skb);
> + int err;
> +
> + __skb_push(skb, offset);
> + err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
> + if (unlikely(err))
> + goto pull;
> +
> + skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
> +
> + vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
> + *vlan_tci = ntohs(vhdr->h_vlan_TCI);
> +
> + memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
> + __skb_pull(skb, VLAN_HLEN);
> +
> + vlan_set_encap_proto(skb, vhdr);
> + skb->mac_header += VLAN_HLEN;
> +
> + if (skb_network_offset(skb) < ETH_HLEN)
> + skb_set_network_header(skb, ETH_HLEN);
> +
> + skb_reset_mac_len(skb);
skb_reset_mac_len() sets length according to ethernet and network
offsets, but mpls expects mac-length to be offset to mpls header (ref.
skb_mpls_header()). Therefore rather than reset we need to subtract
VLAN_HLEN from mac_len. Same goes for vlan-push(), we can add
VLAN_HLEN.
> +pull:
> + __skb_pull(skb, offset);
> +
> + return err;
> +}
> +
> +int skb_vlan_pop(struct sk_buff *skb)
> +{
> + u16 vlan_tci;
> + __be16 vlan_proto;
> + int err;
> +
> + if (likely(vlan_tx_tag_present(skb))) {
> + skb->vlan_tci = 0;
> + } else {
> + if (unlikely((skb->protocol != htons(ETH_P_8021Q) &&
> + skb->protocol != htons(ETH_P_8021AD)) ||
> + skb->len < VLAN_ETH_HLEN))
> + return 0;
> +
> + err = __skb_vlan_pop(skb, &vlan_tci);
> + if (err)
> + return err;
> + }
> + /* move next vlan tag to hw accel tag */
> + if (likely((skb->protocol != htons(ETH_P_8021Q) &&
> + skb->protocol != htons(ETH_P_8021AD)) ||
> + skb->len < VLAN_ETH_HLEN))
> + return 0;
> +
> + vlan_proto = skb->protocol;
> + err = __skb_vlan_pop(skb, &vlan_tci);
> + if (unlikely(err))
> + return err;
> +
> + __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
> + return 0;
> +}
> +EXPORT_SYMBOL(skb_vlan_pop);
> +
> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
> +{
> + if (vlan_tx_tag_present(skb)) {
> + unsigned int offset = skb->data - skb_mac_header(skb);
> + int err;
> +
> + /* __vlan_insert_tag expect skb->data pointing to mac header.
> + * So change skb->data before calling it and change back to
> + * original position later
> + */
> + __skb_push(skb, offset);
> + err = __vlan_insert_tag(skb, skb->vlan_proto,
> + vlan_tx_tag_get(skb));
> + if (err)
> + return err;
> + skb->protocol = skb->vlan_proto;
> + skb_reset_mac_len(skb);
> + __skb_pull(skb, offset);
> +
> + if (skb->ip_summed == CHECKSUM_COMPLETE)
> + skb->csum = csum_add(skb->csum, csum_partial(skb->data
> + + (2 * ETH_ALEN), VLAN_HLEN, 0));
> + }
> + __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
> + return 0;
> +}
> +EXPORT_SYMBOL(skb_vlan_push);
> +
> /**
> * alloc_skb_with_frags - allocate skb with page frags
> *
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 7ffa377..ae7e1b2 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -206,93 +206,27 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
> return 0;
> }
>
> -/* remove VLAN header from packet and update csum accordingly. */
> -static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
> -{
> - struct vlan_hdr *vhdr;
> - int err;
> -
> - err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
> - if (unlikely(err))
> - return err;
> -
> - skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
> -
> - vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
> - *current_tci = vhdr->h_vlan_TCI;
> -
> - memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
> - __skb_pull(skb, VLAN_HLEN);
> -
> - vlan_set_encap_proto(skb, vhdr);
> - skb->mac_header += VLAN_HLEN;
> -
> - if (skb_network_offset(skb) < ETH_HLEN)
> - skb_set_network_header(skb, ETH_HLEN);
> -
> - /* Update mac_len for subsequent MPLS actions */
> - skb_reset_mac_len(skb);
> - return 0;
> -}
> -
> static int pop_vlan(struct sk_buff *skb, struct sw_flow_key *key)
> {
> - __be16 tci;
> int err;
>
> - if (likely(vlan_tx_tag_present(skb))) {
> - skb->vlan_tci = 0;
> - } else {
> - if (unlikely(skb->protocol != htons(ETH_P_8021Q) ||
> - skb->len < VLAN_ETH_HLEN))
> - return 0;
> -
> - err = __pop_vlan_tci(skb, &tci);
> - if (err)
> - return err;
> - }
> - /* move next vlan tag to hw accel tag */
> - if (likely(skb->protocol != htons(ETH_P_8021Q) ||
> - skb->len < VLAN_ETH_HLEN)) {
> + err = skb_vlan_pop(skb);
> + if (vlan_tx_tag_present(skb))
> + invalidate_flow_key(key);
> + else
> key->eth.tci = 0;
> - return 0;
> - }
> -
> - invalidate_flow_key(key);
> - err = __pop_vlan_tci(skb, &tci);
> - if (unlikely(err))
> - return err;
> -
> - __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(tci));
> - return 0;
> + return err;
> }
>
> static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
> const struct ovs_action_push_vlan *vlan)
> {
> - if (unlikely(vlan_tx_tag_present(skb))) {
> - u16 current_tag;
> -
> - /* push down current VLAN tag */
> - current_tag = vlan_tx_tag_get(skb);
> -
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - current_tag);
> - if (!skb)
> - return -ENOMEM;
> - /* Update mac_len for subsequent MPLS actions */
> - skb->mac_len += VLAN_HLEN;
> -
> - if (skb->ip_summed == CHECKSUM_COMPLETE)
> - skb->csum = csum_add(skb->csum, csum_partial(skb->data
> - + (2 * ETH_ALEN), VLAN_HLEN, 0));
> -
> + if (vlan_tx_tag_present(skb))
> invalidate_flow_key(key);
> - } else {
> + else
> key->eth.tci = vlan->vlan_tci;
> - }
> - __vlan_hwaccel_put_tag(skb, vlan->vlan_tpid, ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
> - return 0;
> + return skb_vlan_push(skb, vlan->vlan_tpid,
> + ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
> }
>
> static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
> @@ -858,8 +792,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>
> case OVS_ACTION_ATTR_PUSH_VLAN:
> err = push_vlan(skb, key, nla_data(a));
> - if (unlikely(err)) /* skb already freed. */
> + if (unlikely(err)) {
> + dev_kfree_skb_any(skb);
> return err;
> + }
There is no need to check for error. Just break is sufficient. Error
checking is done after switch case.
> break;
>
> case OVS_ACTION_ATTR_POP_VLAN:
> --
> 1.9.3
>
^ permalink raw reply
* Re: [patch net-next v3 6/9] vlan: introduce __vlan_insert_tag helper which does not free skb
From: Pravin Shelar @ 2014-11-19 8:05 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Jamal Hadi Salim, Tom Herbert, Eric Dumazet,
Willem de Bruijn, Daniel Borkmann, mst, fw, Paul.Durrant,
Thomas Graf, Cong Wang
In-Reply-To: <1416346664-9290-7-git-send-email-jiri@resnulli.us>
On Tue, Nov 18, 2014 at 1:37 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> There's a need for helper which inserts vlan tag but does not free the
> skb in case of an error.
>
> Suggested-by: Pravin Shelar <pshelar@nicira.com>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
> ---
> include/linux/if_vlan.h | 45 ++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 291e670..515a35e 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -282,28 +282,24 @@ static inline bool vlan_hw_offload_capable(netdev_features_t features,
> }
>
> /**
> - * vlan_insert_tag - regular VLAN tag inserting
> + * __vlan_insert_tag - regular VLAN tag inserting
> * @skb: skbuff to tag
> * @vlan_proto: VLAN encapsulation protocol
> * @vlan_tci: VLAN TCI to insert
> *
> * Inserts the VLAN tag into @skb as part of the payload
> - * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
> - *
> - * Following the skb_unshare() example, in case of error, the calling function
> - * doesn't have to worry about freeing the original skb.
> + * Returns error if skb_cow_head failes.
> *
> * Does not change skb->protocol so this function can be used during receive.
> */
> -static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
> - __be16 vlan_proto, u16 vlan_tci)
> +static inline int __vlan_insert_tag(struct sk_buff *skb,
> + __be16 vlan_proto, u16 vlan_tci)
> {
> struct vlan_ethhdr *veth;
>
> - if (skb_cow_head(skb, VLAN_HLEN) < 0) {
> - dev_kfree_skb_any(skb);
> - return NULL;
> - }
> + if (skb_cow_head(skb, VLAN_HLEN) < 0)
> + return -ENOMEM;
> +
> veth = (struct vlan_ethhdr *)skb_push(skb, VLAN_HLEN);
>
> /* Move the mac addresses to the beginning of the new header. */
> @@ -316,6 +312,33 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
> /* now, the TCI */
> veth->h_vlan_TCI = htons(vlan_tci);
>
> + return 0;
> +}
> +
> +/**
> + * vlan_insert_tag - regular VLAN tag inserting
> + * @skb: skbuff to tag
> + * @vlan_proto: VLAN encapsulation protocol
> + * @vlan_tci: VLAN TCI to insert
> + *
> + * Inserts the VLAN tag into @skb as part of the payload
> + * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
> + *
> + * Following the skb_unshare() example, in case of error, the calling function
> + * doesn't have to worry about freeing the original skb.
> + *
> + * Does not change skb->protocol so this function can be used during receive.
> + */
> +static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
> + __be16 vlan_proto, u16 vlan_tci)
> +{
> + int err;
> +
> + err = __vlan_insert_tag(skb, vlan_proto, vlan_tci);
> + if (err) {
> + dev_kfree_skb_any(skb);
> + return NULL;
> + }
> return skb;
> }
>
> --
> 1.9.3
>
^ permalink raw reply
* Re: [patch net-next v3 5/9] vlan: introduce *vlan_hwaccel_push_inside helpers
From: Pravin Shelar @ 2014-11-19 8:05 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Jamal Hadi Salim, Tom Herbert, Eric Dumazet,
Willem de Bruijn, Daniel Borkmann, mst, fw, Paul.Durrant,
Thomas Graf, Cong Wang
In-Reply-To: <1416346664-9290-6-git-send-email-jiri@resnulli.us>
On Tue, Nov 18, 2014 at 1:37 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Use them to push skb->vlan_tci into the payload and avoid code
> duplication.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> drivers/net/vxlan.c | 22 ++++++----------------
> include/linux/if_vlan.h | 34 ++++++++++++++++++++++++++++++++++
> net/core/dev.c | 8 ++------
> net/core/netpoll.c | 4 +---
> net/ipv4/geneve.c | 11 +++--------
> net/openvswitch/datapath.c | 6 +-----
> net/openvswitch/vport-gre.c | 12 ++++--------
> 7 files changed, 51 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index bb8fbab..64d45fa 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1599,14 +1599,9 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
> if (unlikely(err))
> return err;
>
> - if (vlan_tx_tag_present(skb)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> - if (WARN_ON(!skb))
> - return -ENOMEM;
> -
> - skb->vlan_tci = 0;
> - }
> + skb = vlan_hwaccel_push_inside(skb);
> + if (WARN_ON(!skb))
> + return -ENOMEM;
>
> vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
> vxh->vx_flags = htonl(VXLAN_FLAGS);
> @@ -1643,14 +1638,9 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
> if (unlikely(err))
> return err;
>
> - if (vlan_tx_tag_present(skb)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> - if (WARN_ON(!skb))
> - return -ENOMEM;
> -
> - skb->vlan_tci = 0;
> - }
> + skb = vlan_hwaccel_push_inside(skb);
> + if (WARN_ON(!skb))
> + return -ENOMEM;
>
> vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
> vxh->vx_flags = htonl(VXLAN_FLAGS);
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 46e4a15..291e670 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -341,6 +341,40 @@ static inline struct sk_buff *vlan_insert_tag_set_proto(struct sk_buff *skb,
> return skb;
> }
>
> +/*
> + * __vlan_hwaccel_push_inside - pushes vlan tag to the payload
> + * @skb: skbuff to tag
> + *
> + * Pushes the VLAN tag from @skb->vlan_tci inside to the payload.
> + *
> + * Following the skb_unshare() example, in case of error, the calling function
> + * doesn't have to worry about freeing the original skb.
> + */
> +static inline struct sk_buff *__vlan_hwaccel_push_inside(struct sk_buff *skb)
> +{
> + skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> + vlan_tx_tag_get(skb));
> + if (likely(skb))
> + skb->vlan_tci = 0;
> + return skb;
> +}
> +/*
> + * vlan_hwaccel_push_inside - pushes vlan tag to the payload
> + * @skb: skbuff to tag
> + *
> + * Checks is tag is present in @skb->vlan_tci and if it is, it pushes the
> + * VLAN tag from @skb->vlan_tci inside to the payload.
> + *
> + * Following the skb_unshare() example, in case of error, the calling function
> + * doesn't have to worry about freeing the original skb.
> + */
> +static inline struct sk_buff *vlan_hwaccel_push_inside(struct sk_buff *skb)
> +{
> + if (vlan_tx_tag_present(skb))
> + skb = __vlan_hwaccel_push_inside(skb);
> + return skb;
> +}
> +
> /**
> * __vlan_hwaccel_put_tag - hardware accelerated VLAN inserting
> * @skb: skbuff to tag
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 3611e60..ac48362 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2644,12 +2644,8 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
> netdev_features_t features)
> {
> if (vlan_tx_tag_present(skb) &&
> - !vlan_hw_offload_capable(features, skb->vlan_proto)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> - if (skb)
> - skb->vlan_tci = 0;
> - }
> + !vlan_hw_offload_capable(features, skb->vlan_proto))
> + skb = __vlan_hwaccel_push_inside(skb);
> return skb;
> }
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 65d3723..e0ad5d1 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -79,8 +79,7 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
>
> if (vlan_tx_tag_present(skb) &&
> !vlan_hw_offload_capable(features, skb->vlan_proto)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> + skb = __vlan_hwaccel_push_inside(skb);
> if (unlikely(!skb)) {
> /* This is actually a packet drop, but we
> * don't want the code that calls this
> @@ -88,7 +87,6 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
> */
> goto out;
> }
> - skb->vlan_tci = 0;
> }
>
> status = netdev_start_xmit(skb, dev, txq, false);
> diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
> index fd430a6..a457232 100644
> --- a/net/ipv4/geneve.c
> +++ b/net/ipv4/geneve.c
> @@ -131,14 +131,9 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
> if (unlikely(err))
> return err;
>
> - if (vlan_tx_tag_present(skb)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> - if (unlikely(!skb)
> - return -ENOMEM;
> -
> - skb->vlan_tci = 0;
> - }
> + skb = vlan_hwaccel_push_inside(skb);
> + if (unlikely(!skb))
> + return -ENOMEM;
>
> gnvh = (struct genevehdr *)__skb_push(skb, sizeof(*gnvh) + opt_len);
> geneve_build_header(gnvh, tun_flags, vni, opt_len, opt);
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index c63e60e..0cb9d4f 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -425,12 +425,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
> if (!nskb)
> return -ENOMEM;
>
> - nskb = vlan_insert_tag_set_proto(nskb, nskb->vlan_proto,
> - vlan_tx_tag_get(nskb));
> - if (!nskb)
> - return -ENOMEM;
> + nskb = __vlan_hwaccel_push_inside(nskb);
>
> - nskb->vlan_tci = 0;
Need to check for returned nskb for NULL case.
> skb = nskb;
> }
>
> diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
> index 777cd8c..6b69df5 100644
> --- a/net/openvswitch/vport-gre.c
> +++ b/net/openvswitch/vport-gre.c
> @@ -175,14 +175,10 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
> goto err_free_rt;
> }
>
> - if (vlan_tx_tag_present(skb)) {
> - skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> - if (unlikely(!skb) {
> - err = -ENOMEM;
> - goto err_free_rt;
> - }
> - skb->vlan_tci = 0;
> + skb = vlan_hwaccel_push_inside(skb);
> + if (unlikely(!skb)) {
> + err = -ENOMEM;
> + goto err_free_rt;
> }
>
> /* Push Tunnel header. */
> --
> 1.9.3
>
^ permalink raw reply
* Re: [patch net-next v3 4/9] vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto
From: Pravin Shelar @ 2014-11-19 8:05 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Jamal Hadi Salim, Tom Herbert, Eric Dumazet,
Willem de Bruijn, Daniel Borkmann, mst, fw, Paul.Durrant,
Thomas Graf, Cong Wang
In-Reply-To: <1416346664-9290-5-git-send-email-jiri@resnulli.us>
On Tue, Nov 18, 2014 at 1:37 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Name fits better. Plus there's going to be introduced
> __vlan_insert_tag later on.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
> ---
> drivers/net/bonding/bond_main.c | 4 ++--
> drivers/net/ethernet/emulex/benet/be_main.c | 6 ++++--
> drivers/net/vxlan.c | 12 ++++++------
> include/linux/if_vlan.h | 8 +++++---
> net/bridge/br_vlan.c | 4 ++--
> net/core/dev.c | 4 ++--
> net/core/netpoll.c | 4 ++--
> net/ipv4/geneve.c | 11 +++++------
> net/openvswitch/actions.c | 4 +++-
> net/openvswitch/datapath.c | 3 ++-
> net/openvswitch/vport-gre.c | 6 +++---
> 11 files changed, 36 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index e26c682..c1d7da4 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2146,8 +2146,8 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op,
>
> netdev_dbg(slave_dev, "inner tag: proto %X vid %X\n",
> ntohs(outer_tag->vlan_proto), tags->vlan_id);
> - skb = __vlan_put_tag(skb, tags->vlan_proto,
> - tags->vlan_id);
> + skb = vlan_insert_tag_set_proto(skb, tags->vlan_proto,
> + tags->vlan_id);
> if (!skb) {
> net_err_ratelimited("failed to insert inner VLAN tag\n");
> return;
> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
> index 54160cc..d02fbc7 100644
> --- a/drivers/net/ethernet/emulex/benet/be_main.c
> +++ b/drivers/net/ethernet/emulex/benet/be_main.c
> @@ -887,7 +887,8 @@ static struct sk_buff *be_insert_vlan_in_pkt(struct be_adapter *adapter,
> }
>
> if (vlan_tag) {
> - skb = __vlan_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
> + skb = vlan_insert_tag_set_proto(skb, htons(ETH_P_8021Q),
> + vlan_tag);
> if (unlikely(!skb))
> return skb;
> skb->vlan_tci = 0;
> @@ -896,7 +897,8 @@ static struct sk_buff *be_insert_vlan_in_pkt(struct be_adapter *adapter,
> /* Insert the outer VLAN, if any */
> if (adapter->qnq_vid) {
> vlan_tag = adapter->qnq_vid;
> - skb = __vlan_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
> + skb = vlan_insert_tag_set_proto(skb, htons(ETH_P_8021Q),
> + vlan_tag);
> if (unlikely(!skb))
> return skb;
> if (skip_hw_vlan)
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 23b1e8c..bb8fbab 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1600,9 +1600,9 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
> return err;
>
> if (vlan_tx_tag_present(skb)) {
> - if (WARN_ON(!__vlan_put_tag(skb,
> - skb->vlan_proto,
> - vlan_tx_tag_get(skb))))
> + skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> + vlan_tx_tag_get(skb));
> + if (WARN_ON(!skb))
> return -ENOMEM;
>
> skb->vlan_tci = 0;
> @@ -1644,9 +1644,9 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
> return err;
>
> if (vlan_tx_tag_present(skb)) {
> - if (WARN_ON(!__vlan_put_tag(skb,
> - skb->vlan_proto,
> - vlan_tx_tag_get(skb))))
> + skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> + vlan_tx_tag_get(skb));
> + if (WARN_ON(!skb))
> return -ENOMEM;
>
> skb->vlan_tci = 0;
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index 75b70a5..46e4a15 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -320,8 +320,9 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
> }
>
> /**
> - * __vlan_put_tag - regular VLAN tag inserting
> + * vlan_insert_tag_set_proto - regular VLAN tag inserting
> * @skb: skbuff to tag
> + * @vlan_proto: VLAN encapsulation protocol
> * @vlan_tci: VLAN TCI to insert
> *
> * Inserts the VLAN tag into @skb as part of the payload
> @@ -330,8 +331,9 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
> * Following the skb_unshare() example, in case of error, the calling function
> * doesn't have to worry about freeing the original skb.
> */
> -static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb,
> - __be16 vlan_proto, u16 vlan_tci)
> +static inline struct sk_buff *vlan_insert_tag_set_proto(struct sk_buff *skb,
> + __be16 vlan_proto,
> + u16 vlan_tci)
> {
> skb = vlan_insert_tag(skb, vlan_proto, vlan_tci);
> if (skb)
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 150048f..97b8ddf 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -199,8 +199,8 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
> if (skb->vlan_proto != proto) {
> /* Protocol-mismatch, empty out vlan_tci for new tag */
> skb_push(skb, ETH_HLEN);
> - skb = __vlan_put_tag(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> + skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> + vlan_tx_tag_get(skb));
> if (unlikely(!skb))
> return false;
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 1ab168e..3611e60 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2645,8 +2645,8 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
> {
> if (vlan_tx_tag_present(skb) &&
> !vlan_hw_offload_capable(features, skb->vlan_proto)) {
> - skb = __vlan_put_tag(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> + skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> + vlan_tx_tag_get(skb));
> if (skb)
> skb->vlan_tci = 0;
> }
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index e6645b4..65d3723 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -79,8 +79,8 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
>
> if (vlan_tx_tag_present(skb) &&
> !vlan_hw_offload_capable(features, skb->vlan_proto)) {
> - skb = __vlan_put_tag(skb, skb->vlan_proto,
> - vlan_tx_tag_get(skb));
> + skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> + vlan_tx_tag_get(skb));
> if (unlikely(!skb)) {
> /* This is actually a packet drop, but we
> * don't want the code that calls this
> diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
> index 31802af..fd430a6 100644
> --- a/net/ipv4/geneve.c
> +++ b/net/ipv4/geneve.c
> @@ -132,12 +132,11 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
> return err;
>
> if (vlan_tx_tag_present(skb)) {
> - if (unlikely(!__vlan_put_tag(skb,
> - skb->vlan_proto,
> - vlan_tx_tag_get(skb)))) {
> - err = -ENOMEM;
> - return err;
> - }
> + skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> + vlan_tx_tag_get(skb));
> + if (unlikely(!skb)
> + return -ENOMEM;
> +
> skb->vlan_tci = 0;
> }
>
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 749a301..426b913 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -287,7 +287,9 @@ static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
> /* push down current VLAN tag */
> current_tag = vlan_tx_tag_get(skb);
>
> - if (!__vlan_put_tag(skb, skb->vlan_proto, current_tag))
> + skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> + current_tag);
> + if (!skb)
> return -ENOMEM;
> /* Update mac_len for subsequent MPLS actions */
> skb->mac_len += VLAN_HLEN;
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index ab141d4..c63e60e 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -425,7 +425,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
> if (!nskb)
> return -ENOMEM;
>
> - nskb = __vlan_put_tag(nskb, nskb->vlan_proto, vlan_tx_tag_get(nskb));
> + nskb = vlan_insert_tag_set_proto(nskb, nskb->vlan_proto,
> + vlan_tx_tag_get(nskb));
> if (!nskb)
> return -ENOMEM;
>
> diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
> index 8e61a5c..777cd8c 100644
> --- a/net/openvswitch/vport-gre.c
> +++ b/net/openvswitch/vport-gre.c
> @@ -176,9 +176,9 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
> }
>
> if (vlan_tx_tag_present(skb)) {
> - if (unlikely(!__vlan_put_tag(skb,
> - skb->vlan_proto,
> - vlan_tx_tag_get(skb)))) {
> + skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
> + vlan_tx_tag_get(skb));
> + if (unlikely(!skb) {
> err = -ENOMEM;
> goto err_free_rt;
> }
> --
> 1.9.3
>
^ permalink raw reply
* Re: [patch net-next v3 2/9] vlan: make __vlan_hwaccel_put_tag return void
From: Pravin Shelar @ 2014-11-19 8:05 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Jamal Hadi Salim, Tom Herbert, Eric Dumazet,
Willem de Bruijn, Daniel Borkmann, mst, fw, Paul.Durrant,
Thomas Graf, Cong Wang
In-Reply-To: <1416346664-9290-3-git-send-email-jiri@resnulli.us>
On Tue, Nov 18, 2014 at 1:37 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Always returns the same skb it gets, so change to void.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
> ---
> drivers/scsi/fcoe/fcoe.c | 6 ++----
> include/linux/if_vlan.h | 9 ++++-----
> net/8021q/vlan_dev.c | 2 +-
> 3 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index 4a8ac7d..73a8cc4 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -1669,10 +1669,8 @@ static int fcoe_xmit(struct fc_lport *lport, struct fc_frame *fp)
> fcoe->realdev->features & NETIF_F_HW_VLAN_CTAG_TX) {
> /* must set skb->dev before calling vlan_put_tag */
> skb->dev = fcoe->realdev;
> - skb = __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
> - vlan_dev_vlan_id(fcoe->netdev));
> - if (!skb)
> - return -ENOMEM;
> + __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
> + vlan_dev_vlan_id(fcoe->netdev));
> } else
> skb->dev = fcoe->netdev;
>
> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
> index d69f057..1b5dbc2 100644
> --- a/include/linux/if_vlan.h
> +++ b/include/linux/if_vlan.h
> @@ -347,13 +347,11 @@ static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb,
> *
> * Puts the VLAN TCI in @skb->vlan_tci and lets the device do the rest
> */
> -static inline struct sk_buff *__vlan_hwaccel_put_tag(struct sk_buff *skb,
> - __be16 vlan_proto,
> - u16 vlan_tci)
> +static inline void __vlan_hwaccel_put_tag(struct sk_buff *skb,
> + __be16 vlan_proto, u16 vlan_tci)
> {
> skb->vlan_proto = vlan_proto;
> skb->vlan_tci = VLAN_TAG_PRESENT | vlan_tci;
> - return skb;
> }
>
> /**
> @@ -368,7 +366,8 @@ static inline struct sk_buff *vlan_put_tag(struct sk_buff *skb,
> __be16 vlan_proto, u16 vlan_tci)
> {
> if (vlan_hw_offload_capable(skb->dev->features, vlan_proto)) {
> - return __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
> + __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
> + return skb;
> } else {
> return __vlan_put_tag(skb, vlan_proto, vlan_tci);
> }
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 0d441ec..d6524b2 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -150,7 +150,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
> u16 vlan_tci;
> vlan_tci = vlan->vlan_id;
> vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
> - skb = __vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
> + __vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
> }
>
> skb->dev = vlan->real_dev;
> --
> 1.9.3
>
^ permalink raw reply
* Re: Device Tree Binding for Marvell DSA Switch on imx28 board over Mdio Interface
From: Oliver Graute @ 2014-11-19 7:49 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, netdev@vger.kernel.org, buytenh
In-Reply-To: <546B8E92.7080000@gmail.com>
On Tue, Nov 18, 2014 at 7:23 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 11/18/2014 12:30 AM, Oliver Graute wrote:
>>> Hi Oliver
>>>
>>> How do you have the strapping pins on the switch set? They determine
>>> what address on the mdio bus the chip responds to.
>>
>> On the circuit diagram the PIN 54 (P5_IND1/P5ID1) is set to
>> "Configuration Address: 0101"
>> P5_MODE[3:0]=0111 = Single RMII MAC Mode (100Mbps FD with 50 MHz clock input)
>> PIN 59 R1_LED/NO_CPU Configuration: CPU is attached SMI address is 0x10 to 0x1F
>>
>> But what is the mdio address of the whole switch? or can I only
>> address individual phy ports?
>
> You should specify in the Device Tree the switch pseudo-PHY address,
> typically 16 for Marvell switches. You can still access the individual
> ports' PHY addresses using address 0 through N.
>
How do I do that exactly? do you have an example Device Tree Snippet?
Best regards,
Oliver
^ permalink raw reply
* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: lucien xin @ 2014-11-19 7:47 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: network dev, Steffen Klassert
In-Reply-To: <546B8093.4080507@6wind.com>
>> ---
>> net/ipv4/ip_vti.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
>> index 3e86101..1a7e979 100644
>> --- a/net/ipv4/ip_vti.c
>> +++ b/net/ipv4/ip_vti.c
>> @@ -528,6 +528,7 @@ static struct rtnl_link_ops vti_link_ops __read_mostly
>> = {
>> .validate = vti_tunnel_validate,
>> .newlink = vti_newlink,
>> .changelink = vti_changelink,
>> + .dellink = ip_tunnel_dellink,
>
> Nitpicking: other lines into this struct uses tabs to align the '=', but
> the one you add uses spaces.
>
>
okay, thanks, I will correct it.
^ permalink raw reply
* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: lucien xin @ 2014-11-19 7:44 UTC (permalink / raw)
To: Cong Wang; +Cc: Nicolas Dichtel, network dev, Steffen Klassert
In-Reply-To: <CAHA+R7NjO+bDhN5QX95d8V0VqvbacxCzoHWT5NpDaDn3YCtCeg@mail.gmail.com>
On Wed, Nov 19, 2014 at 7:25 AM, Cong Wang <cwang@twopensource.com> wrote:
> On Tue, Nov 18, 2014 at 9:23 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>>
>> A quick look at the ipv6 side seems to show that there is the same problem.
>> Can
>> you provide the IPv6 patch too?
>>
>
> IPv6 doesn't use ip_tunnel library, so needs to provide its own implementation:
>
> diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
> index ec84d03..3ae094b 100644
> --- a/net/ipv6/ip6_vti.c
> +++ b/net/ipv6/ip6_vti.c
> @@ -911,6 +911,16 @@ static int vti6_newlink(struct net *src_net,
> struct net_device *dev,
> return vti6_tnl_create2(dev);
> }
>
> +static void vti6_dellink(struct net_device *dev, struct list_head *head)
> +{
> + struct net *net = dev_net(dev);
> + struct vti6_net *ip6n = net_generic(net, vti6_net_id);
> +
> + if (dev != ip6n->fb_tnl_dev)
> + unregister_netdevice_queue(dev, head);
> +}
> +
> +
> static int vti6_changelink(struct net_device *dev, struct nlattr *tb[],
> struct nlattr *data[])
> {
> @@ -986,6 +996,7 @@ static struct rtnl_link_ops vti6_link_ops __read_mostly = {
> .setup = vti6_dev_setup,
> .validate = vti6_validate,
> .newlink = vti6_newlink,
> + .dellink = vti6_dellink,
> .changelink = vti6_changelink,
> .get_size = vti6_get_size,
> .fill_info = vti6_fill_info,
the issue do not exist in ip6_vti. cause vti6_init_net() complete the
device ip6_vti0 initialization,
in which dev->rtnl_link_ops never be pointed. so the ip6_vti0's
rtnl_link_ops is null, which lead
to the dellink() will never be invoked in rtnl_dellink():
if (!ops || !ops->dellink)
return -EOPNOTSUPP;
ops->dellink(dev, &list_kill);
unregister_netdevice_many(&list_kill);
instead, "ip link del ip6_vti0" wil return:
RTNETLINK answers: Operation not supported
if this patch for ip6 is to be consistent with ipv4 vti. I suggest we
should also add the below code
to init ip6_vti0's rtnl_link_ops when the ip6_vti0 is inited:
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index 31089d1..e68c099 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -1020,6 +1020,7 @@ static int __net_init vti6_init_net(struct net *net)
if (!ip6n->fb_tnl_dev)
goto err_alloc_dev;
dev_net_set(ip6n->fb_tnl_dev, net);
+ ip6n->fb_tnl_dev->rtnl_link_ops = &vti6_link_ops;
err = vti6_fb_tnl_dev_init(ip6n->fb_tnl_dev);
if (err < 0)
^ permalink raw reply related
* [PATCH net-next v2] enic: support skb->xmit_more
From: Govindarajulu Varadarajan @ 2014-11-19 7:29 UTC (permalink / raw)
To: davem, netdev; +Cc: ssujith, benve, eric.dumazet, Govindarajulu Varadarajan
Check and update posted_index only when skb->xmit_more is 0 or tx queue is full.
v2:
use txq_map instead of skb_get_queue_mapping(skb)
Signed-off-by: Govindarajulu Varadarajan <_govind@gmx.com>
---
drivers/net/ethernet/cisco/enic/enic_main.c | 8 ++++++--
drivers/net/ethernet/cisco/enic/vnic_wq.h | 20 +++++++++++---------
2 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c
index 5afe360..b9cda2f 100644
--- a/drivers/net/ethernet/cisco/enic/enic_main.c
+++ b/drivers/net/ethernet/cisco/enic/enic_main.c
@@ -533,6 +533,7 @@ static netdev_tx_t enic_hard_start_xmit(struct sk_buff *skb,
struct vnic_wq *wq;
unsigned long flags;
unsigned int txq_map;
+ struct netdev_queue *txq;
if (skb->len <= 0) {
dev_kfree_skb_any(skb);
@@ -541,6 +542,7 @@ static netdev_tx_t enic_hard_start_xmit(struct sk_buff *skb,
txq_map = skb_get_queue_mapping(skb) % enic->wq_count;
wq = &enic->wq[txq_map];
+ txq = netdev_get_tx_queue(netdev, txq_map);
/* Non-TSO sends must fit within ENIC_NON_TSO_MAX_DESC descs,
* which is very likely. In the off chance it's going to take
@@ -558,7 +560,7 @@ static netdev_tx_t enic_hard_start_xmit(struct sk_buff *skb,
if (vnic_wq_desc_avail(wq) <
skb_shinfo(skb)->nr_frags + ENIC_DESC_MAX_SPLITS) {
- netif_tx_stop_queue(netdev_get_tx_queue(netdev, txq_map));
+ netif_tx_stop_queue(txq);
/* This is a hard error, log it */
netdev_err(netdev, "BUG! Tx ring full when queue awake!\n");
spin_unlock_irqrestore(&enic->wq_lock[txq_map], flags);
@@ -568,7 +570,9 @@ static netdev_tx_t enic_hard_start_xmit(struct sk_buff *skb,
enic_queue_wq_skb(enic, wq, skb);
if (vnic_wq_desc_avail(wq) < MAX_SKB_FRAGS + ENIC_DESC_MAX_SPLITS)
- netif_tx_stop_queue(netdev_get_tx_queue(netdev, txq_map));
+ netif_tx_stop_queue(txq);
+ if (!skb->xmit_more || netif_xmit_stopped(txq))
+ vnic_wq_doorbell(wq);
spin_unlock_irqrestore(&enic->wq_lock[txq_map], flags);
diff --git a/drivers/net/ethernet/cisco/enic/vnic_wq.h b/drivers/net/ethernet/cisco/enic/vnic_wq.h
index 2c6c708..816f1ad 100644
--- a/drivers/net/ethernet/cisco/enic/vnic_wq.h
+++ b/drivers/net/ethernet/cisco/enic/vnic_wq.h
@@ -104,6 +104,17 @@ static inline void *vnic_wq_next_desc(struct vnic_wq *wq)
return wq->to_use->desc;
}
+static inline void vnic_wq_doorbell(struct vnic_wq *wq)
+{
+ /* Adding write memory barrier prevents compiler and/or CPU
+ * reordering, thus avoiding descriptor posting before
+ * descriptor is initialized. Otherwise, hardware can read
+ * stale descriptor fields.
+ */
+ wmb();
+ iowrite32(wq->to_use->index, &wq->ctrl->posted_index);
+}
+
static inline void vnic_wq_post(struct vnic_wq *wq,
void *os_buf, dma_addr_t dma_addr,
unsigned int len, int sop, int eop,
@@ -122,15 +133,6 @@ static inline void vnic_wq_post(struct vnic_wq *wq,
buf->wr_id = wrid;
buf = buf->next;
- if (eop) {
- /* Adding write memory barrier prevents compiler and/or CPU
- * reordering, thus avoiding descriptor posting before
- * descriptor is initialized. Otherwise, hardware can read
- * stale descriptor fields.
- */
- wmb();
- iowrite32(buf->index, &wq->ctrl->posted_index);
- }
wq->to_use = buf;
wq->ring.desc_avail -= desc_skip_cnt;
--
2.1.3
^ permalink raw reply related
* Re: [PATCH net] openvswitch: Fix mask generation for IPv6 labels.
From: Joe Stringer @ 2014-11-19 7:25 UTC (permalink / raw)
To: Pravin Shelar; +Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev, LKML
In-Reply-To: <CALnjE+qrkw92tZ-wgpGOPtbuMJVVCfo--7w6PAGO6OW4skiGQQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 18 November 2014 22:09, Pravin Shelar <pshelar@nicira.com> wrote:
> On Tue, Nov 18, 2014 at 10:54 AM, Joe Stringer <joestringer@nicira.com>
> wrote:
> > When userspace doesn't provide a mask, OVS datapath generates a fully
> > unwildcarded mask for the flow. This is done by taking a copy of the
> > flow key, then iterating across its attributes, setting all values to
> > 0xff. This works for most attributes, as the length of the netlink
> > attribute typically matches the length of the value. However, IPv6
> > labels only use the lower 20 bits of the field. This patch makes a
> > special case to handle this.
> >
> > This fixes the following error seen when installing IPv6 flows without a
> mask:
> >
> > openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff,
> max=fffff)
> >
> We should allow exact match mask here rather than generating
> wildcarded mask. So that ovs can catch invalid ipv6.label.
I don't quite follow, I thought this was exact-match? (The existing
function sets all bits to 1)
In this case, userspace has not specified a mask, but the kernel complains
about a mask that is too wide (because it generated a mask that's too
wide). Do you have an alternative fix in mind?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* [PATCH V2 net] virtio-net: validate features during probe
From: Jason Wang @ 2014-11-19 7:21 UTC (permalink / raw)
To: rusty, mst, virtualization, netdev, linux-kernel
This patch validates feature dependencies during probe and fail the probing
if a dependency is missed. This fixes the issues of hitting BUG()
when qemu fails to advertise features correctly. One example is booting
guest with ctrl_vq=off through qemu.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
Changes from V1:
- Drop VIRTIO_NET_F_*_UFO from the checklist, since it was disabled
---
drivers/net/virtio_net.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 91 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ec2a8b4..b16a761 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1673,6 +1673,93 @@ static const struct attribute_group virtio_net_mrg_rx_group = {
};
#endif
+static int virtnet_validate_features(struct virtio_device *dev,
+ unsigned int *table,
+ int table_size,
+ unsigned int feature)
+{
+ int i;
+
+ if (!virtio_has_feature(dev, feature)) {
+ for (i = 0; i < table_size; i++) {
+ unsigned int f = table[i];
+
+ if (virtio_has_feature(dev, f)) {
+ dev_err(&dev->dev,
+ "buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x was not",
+ f, feature);
+ return -EINVAL;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int virtnet_check_features(struct virtio_device *dev)
+{
+ unsigned int features_for_ctrl_vq[] = {
+ VIRTIO_NET_F_CTRL_RX,
+ VIRTIO_NET_F_CTRL_VLAN,
+ VIRTIO_NET_F_GUEST_ANNOUNCE,
+ VIRTIO_NET_F_MQ,
+ VIRTIO_NET_F_CTRL_MAC_ADDR
+ };
+ unsigned int features_for_guest_csum[] = {
+ VIRTIO_NET_F_GUEST_TSO4,
+ VIRTIO_NET_F_GUEST_TSO6,
+ VIRTIO_NET_F_GUEST_ECN,
+ };
+ unsigned int features_for_host_csum[] = {
+ VIRTIO_NET_F_HOST_TSO4,
+ VIRTIO_NET_F_HOST_TSO6,
+ VIRTIO_NET_F_HOST_ECN,
+ };
+ int err;
+
+ err = virtnet_validate_features(dev, features_for_ctrl_vq,
+ ARRAY_SIZE(features_for_ctrl_vq),
+ VIRTIO_NET_F_CTRL_VQ);
+ if (err)
+ return err;
+
+ err = virtnet_validate_features(dev, features_for_guest_csum,
+ ARRAY_SIZE(features_for_guest_csum),
+ VIRTIO_NET_F_GUEST_CSUM);
+ if (err)
+ return err;
+
+ err = virtnet_validate_features(dev, features_for_host_csum,
+ ARRAY_SIZE(features_for_host_csum),
+ VIRTIO_NET_F_CSUM);
+ if (err)
+ return err;
+
+ if (virtio_has_feature(dev, VIRTIO_NET_F_GUEST_ECN) &&
+ (!virtio_has_feature(dev, VIRTIO_NET_F_GUEST_TSO4) ||
+ !virtio_has_feature(dev, VIRTIO_NET_F_GUEST_TSO6))) {
+ dev_err(&dev->dev,
+ "buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x or 0x%x was not",
+ VIRTIO_NET_F_GUEST_ECN,
+ VIRTIO_NET_F_GUEST_TSO4,
+ VIRTIO_NET_F_GUEST_TSO6);
+ return -EINVAL;
+ }
+
+ if (virtio_has_feature(dev, VIRTIO_NET_F_HOST_ECN) &&
+ (!virtio_has_feature(dev, VIRTIO_NET_F_HOST_TSO4) ||
+ !virtio_has_feature(dev, VIRTIO_NET_F_HOST_TSO6))) {
+ dev_err(&dev->dev,
+ "buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x or 0x%x was not",
+ VIRTIO_NET_F_HOST_ECN,
+ VIRTIO_NET_F_HOST_TSO4,
+ VIRTIO_NET_F_HOST_TSO6);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int virtnet_probe(struct virtio_device *vdev)
{
int i, err;
@@ -1680,6 +1767,10 @@ static int virtnet_probe(struct virtio_device *vdev)
struct virtnet_info *vi;
u16 max_queue_pairs;
+ err = virtnet_check_features(vdev);
+ if (err)
+ return -EINVAL;
+
/* Find if host supports multiqueue virtio_net device */
err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
struct virtio_net_config,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic
From: lucien xin @ 2014-11-19 7:10 UTC (permalink / raw)
To: Cong Wang; +Cc: network dev
In-Reply-To: <CAHA+R7N7x5eMKFtyWpOcM_FGJ+9h-K+wLEwzpZya1kiztUo8RQ@mail.gmail.com>
>>
>
> Hmm, your fix is correct, but the description is wrong, we should just skip
> the unregister of fb tunnel device like other tunnels. Without ->dellink()
> the fb device is still removed from the global netdev list, this needs to skip,
> otherwise ip_vti0 will disappear from your system.
yes, you are right. the issue only exist in the fb tunnel device, I will change
the description.
^ permalink raw reply
* Re: [PATCH net] virtio-net: validate features during probe
From: Jason Wang @ 2014-11-19 7:07 UTC (permalink / raw)
To: rusty, mst, virtualization, netdev, linux-kernel
In-Reply-To: <1416378939-28821-1-git-send-email-jasowang@redhat.com>
On 11/19/2014 02:35 PM, Jason Wang wrote:
> This patch validates feature dependencies during probe and fail the probing
> if a dependency is missed. This fixes the issues of hitting BUG()
> when qemu fails to advertise features correctly. One example is booting
> guest with ctrl_vq=off through qemu.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
Just notice UFO has been disabled after
3d0ad09412ffe00c9afa201d01effdb6023d09b4 "drivers/net: Disable UFO
through virtio". Will post V2 and drop VIRTIO_NET_F_*_UFO from the
checklist.
^ permalink raw reply
* Re: [PATCH] bonding: clear header_ops when last slave detached (v2)
From: Wengang @ 2014-11-19 7:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1416375565.14060.43.camel@edumazet-glaptop2.roam.corp.google.com>
Hi Eric,
于 2014年11月19日 13:39, Eric Dumazet 写道:
> On Wed, 2014-11-19 at 13:18 +0800, Wengang Wang wrote:
>> When last slave of a bonding master is removed, the bonding then does not work.
>> When packet_snd is called against with a master net_device, it accesses
>> header_ops. In case the header_ops is not valid any longer(say ipoib module
>> unloaded), it will then access an invalid memory address.
>> This patch try to fix this issue by clearing header_ops when last slave
>> detached.
>>
>> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
>> ---
>> drivers/net/bonding/bond_main.c | 1 +
>> 1 file changed, 1 insertion(+)
> It seems you basically ignored my feedback.
>
>
>
> Some code is doing :
>
> [A] if (dev->header_ops) {
> ...
> [B] dev->header_ops->XXX()
>
>
>
> Nothing prevents you doing the clear after [A] , and before [B]
>
Yes, that's true. So the simplest way is move ipoib_header_ops to vmlinux.
thanks for review.
wengang
^ permalink raw reply
* Re: [BNX2] A Netdev Watchdog with kernel stable 3.4
From: Michael Chan @ 2014-11-19 6:58 UTC (permalink / raw)
To: Rui Xiang, sony.chacko; +Cc: netdev
In-Reply-To: <546C388B.2050603@huawei.com>
Copying the current maintainer Sony. The PCI command register looks
strange. Please see below.
On Wed, 2014-11-19 at 14:28 +0800, Rui Xiang wrote:
> ping...
>
> On 2014/11/17 20:42, Rui Xiang wrote:
> > Hi Michael,
> >
> > On a system that was running stable 3.4.87, I got the below stack.
> > That was a NETDEV WATCHDOG. And we could also see watchdog timeouts with the
> > BNX2. (After the stack, an oops occurred while running ifconfig. I think it
> > would be related to this timeout.)
> >
> > Otherwises, the bnx2_dump_state and bnx2_dump_mcp_state have printed the states.
> > Through these states info, can we got the real situation of NIC1.
> > Or can we see what resulted the WATCHDOG, a bnx2 device fault or other reasons.
> >
> > Thanks.
> >
> >
> > *The stack*:
> >
> > WARNING: at /usr/src/packages/BUILD/kernel-default-3.4.87/linux-3.4/net/sched/sch_generic.c:256 dev_watchdog+0x256/0x260()
> > NETDEV WATCHDOG: NIC1 (bnx2): transmit queue 3 timed out
> > Modules linked in: smb3_failover(O) smb2(O) smb(O) smb_manager(O) nfs(O) nfs_acl(O) nfsd(O) lockd(O) nal(O) auth_rpcgss(O)
> > scsi_dh_hp_sw scsi_dh_alua scsi_dh_emc scsi_dh_rdac scsi_dh scsi_mod [last unloaded: ipmi_msghandler]
> > Pid: 0, comm: swapper/0 Tainted: P W O 3.4.87-default #1
> > Call Trace:
> > <IRQ> [<ffffffff8103fcea>] warn_slowpath_common+0x7a/0xb0
> > [<ffffffff8103fdc1>] warn_slowpath_fmt+0x41/0x50
> > [<ffffffff81047749>] ? raise_softirq_irqoff+0x9/0x30
> > [<ffffffff813ae0f6>] dev_watchdog+0x256/0x260
> > [<ffffffff813adea0>] ? dev_deactivate_queue.constprop.30+0x70/0x70
> > [<ffffffff8104edc7>] run_timer_softirq+0x147/0x340
> > [<ffffffff810470d8>] __do_softirq+0xc8/0x1e0
> > [<ffffffff8109250f>] ? tick_program_event+0x1f/0x30
> > [<ffffffff81460a6c>] call_softirq+0x1c/0x30
> > [<ffffffff8100417d>] do_softirq+0x9d/0xd0
> > [<ffffffff810474a5>] irq_exit+0xb5/0xc0
> > [<ffffffff81021b49>] smp_apic_timer_interrupt+0x69/0xa0
> > [<ffffffff8146006f>] apic_timer_interrupt+0x6f/0x80
> > <EOI> [<ffffffff81457bdd>] ? retint_restore_args+0x13/0x13
> > [<ffffffff81360149>] ? poll_idle+0x49/0x90
> > [<ffffffff8136011f>] ? poll_idle+0x1f/0x90
> > [<ffffffff8135fcc9>] cpuidle_enter+0x19/0x20
> > [<ffffffff813602f2>] cpuidle_idle_call+0xa2/0x250
> > [<ffffffff8100b08f>] cpu_idle+0x6f/0xe0
> > [<ffffffff81915960>] ? rawsock_init+0x12/0x12
> > [<ffffffff814331c9>] rest_init+0x6d/0x74
> > [<ffffffff818d3be5>] start_kernel+0x3a2/0x3af
> > [<ffffffff818d3642>] ? repair_env_string+0x5e/0x5e
> > [<ffffffff818d332a>] x86_64_start_reservations+0x131/0x135
> > [<ffffffff818d342e>] x86_64_start_kernel+0x100/0x10f
> > ---[ end trace 497e24e681e0c02d ]---
> > bnx2 0000:05:00.1: NIC1: DEBUG: intr_sem[0] PCI_CMD[00100002]
The memory bit in PCI_CMD is set, but the bus master bit is not set.
DMA won't work if the bus master bit is not set. What was happening
before the timeout? Was it working fine for a while and it suddenly
stopped?
> > bnx2 0000:05:00.1: NIC1: DEBUG: PCI_PM[19002008] PCI_MISC_CFG[92000088]
> > bnx2 0000:05:00.1: NIC1: DEBUG: EMAC_TX_STATUS[00000008] EMAC_RX_STATUS[00000000]
> > bnx2 0000:05:00.1: NIC1: DEBUG: RPM_MGMT_PKT_CTRL[40000088]
> > bnx2 0000:05:00.1: NIC1: DEBUG: HC_STATS_INTERRUPT_STATUS[01ff0000]
> > bnx2 0000:05:00.1: NIC1: DEBUG: PBA[00000000]
> > bnx2 0000:05:00.1: NIC1: <--- start MCP states dump --->
> > bnx2 0000:05:00.1: NIC1: DEBUG: MCP_STATE_P0[0003e10e] MCP_STATE_P1[0003e10e]
> > bnx2 0000:05:00.1: NIC1: DEBUG: MCP mode[0000b800] state[80008000] evt_mask[00000500]
> > bnx2 0000:05:00.1: NIC1: DEBUG: pc[08008f60] pc[0800d21c] instr[00051080]
> > bnx2 0000:05:00.1: NIC1: DEBUG: shmem states:
> > bnx2 0000:05:00.1: NIC1: DEBUG: drv_mb[01030003] fw_mb[00000003] link_status[0000006f] drv_pulse_mb[0000073d]
> > bnx2 0000:05:00.1: NIC1: DEBUG: dev_info_signature[44564907] reset_type[01005254] condition[0003e10e]
> > bnx2 0000:05:00.1: NIC1: DEBUG: 000003cc: 00000000 00000000 00000000 00000000
> > bnx2 0000:05:00.1: NIC1: DEBUG: 000003dc: 00000000 00000000 00000000 00000000
> > bnx2 0000:05:00.1: NIC1: DEBUG: 000003ec: 00000000 00000000 00000000 00000000
> > bnx2 0000:05:00.1: NIC1: DEBUG: 0x3fc[00000000]
> > bnx2 0000:05:00.1: NIC1: <--- end MCP states dump --->
> >
>
>
^ permalink raw reply
* [PATCH net] tcp: fix connect() invalid -EADDRNOTAVAIL error
From: Jon Maxwell @ 2014-11-19 6:37 UTC (permalink / raw)
To: davem
Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel, jmaxwell,
Jon Maxwell
The connect() routine returns -EADDRNOTAVAIL without doing a 4
tuple check when the hash buckets were previously allocated by
bind() and all local ports are used.
The bind() routine creates the local port hash buckets in
inet_csk_get_port(). Depending on the socket options it sets
tb->fastreuse and tb->fastreuseport to 0 or 1 in the bucket.
However the __inet_hash_connect() routine initializes the hash
buckets differently and sets these to -1. The end result is
that connect() calling into __inet_hash_connect() will
subsequently ignore the check_established() routine if, here
__inet_hash_connect()
.
.
if (tb->fastreuse >= 0 ||↩
tb->fastreuseport >= 0)↩
goto next_port;
and cycle through all local ports until it returns -EADDRNOTAVAIL.
The 4 tuple check is in check_established() so connect() can fail
unnecessarily.
Prerequisites for this to happen:
1) The local tcp port range must be exhausted.
2) A process must have called bind() followed by connect() for all
local ports.
3) A different process calls connect() only which returns -EADDRNOTAVAIL.
4) The system more than 1 interface configured.
If a system has 2 IP Addresses and all local tcp ports are in use
for connection from IP Address (1). Connecting to the same ports
via IP Address (2) should work based on the 4 tuple rule. But it
fails under this condition.
To fix this make __inet_hash_connect() honour inet_csk_get_port()'s
tb->fastreuse* variables.
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
net/ipv4/inet_hashtables.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 9111a4e..b39e89e 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -513,8 +513,8 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
inet_bind_bucket_for_each(tb, &head->chain) {
if (net_eq(ib_net(tb), net) &&
tb->port == port) {
- if (tb->fastreuse >= 0 ||
- tb->fastreuseport >= 0)
+ if (tb->fastreuse > 0 ||
+ tb->fastreuseport > 0)
goto next_port;
WARN_ON(hlist_empty(&tb->owners));
if (!check_established(death_row, sk,
@@ -530,8 +530,6 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
spin_unlock(&head->lock);
break;
}
- tb->fastreuse = -1;
- tb->fastreuseport = -1;
goto ok;
next_port:
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next 3/4] igb: enable internal PPS for the i210.
From: Richard Cochran @ 2014-11-19 6:37 UTC (permalink / raw)
To: netdev
Cc: David Miller, bruce.w.allan, Jacob Keller, Jeff Kirsher,
John Ronciak, Matthew Vick
In-Reply-To: <e5b4ef7c56119e2d86e42f5680b1b82c9ec1a38e.1416265321.git.richardcochran@gmail.com>
On Tue, Nov 18, 2014 at 12:06:24AM +0100, Richard Cochran wrote:
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -5386,8 +5386,14 @@ void igb_update_stats(struct igb_adapter *adapter,
> static void igb_tsync_interrupt(struct igb_adapter *adapter)
> {
> struct e1000_hw *hw = &adapter->hw;
> + struct ptp_clock_event event;
> u32 tsicr = rd32(E1000_TSICR);
>
> + if (tsicr & TSINTR_SYS_WRAP) {
> + event.type = PTP_CLOCK_PPS;
> + ptp_clock_event(adapter->ptp_clock, &event);
This causes a BUG for the 82580. When you enable E1000_TSICR_TXTS, it
seems that TSINTR_SYS_WRAP is also automatically enabled. Because the
82580 variant has no pps device enabled, the driver then dereferences
a null pointer.
So we need to make the call to ptp_clock_event() conditional based on
whether pps is enabled. I'll fix this in V2.
> + }
> +
Thanks,
Richard
^ permalink raw reply
* [PATCH net] virtio-net: validate features during probe
From: Jason Wang @ 2014-11-19 6:35 UTC (permalink / raw)
To: rusty, mst, virtualization, netdev, linux-kernel
This patch validates feature dependencies during probe and fail the probing
if a dependency is missed. This fixes the issues of hitting BUG()
when qemu fails to advertise features correctly. One example is booting
guest with ctrl_vq=off through qemu.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ec2a8b4..4a0ad46 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1673,6 +1673,95 @@ static const struct attribute_group virtio_net_mrg_rx_group = {
};
#endif
+static int virtnet_validate_features(struct virtio_device *dev,
+ unsigned int *table,
+ int table_size,
+ unsigned int feature)
+{
+ int i;
+
+ if (!virtio_has_feature(dev, feature)) {
+ for (i = 0; i < table_size; i++) {
+ unsigned int f = table[i];
+
+ if (virtio_has_feature(dev, f)) {
+ dev_err(&dev->dev,
+ "buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x was not",
+ f, feature);
+ return -EINVAL;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int virtnet_check_features(struct virtio_device *dev)
+{
+ unsigned int features_for_ctrl_vq[] = {
+ VIRTIO_NET_F_CTRL_RX,
+ VIRTIO_NET_F_CTRL_VLAN,
+ VIRTIO_NET_F_GUEST_ANNOUNCE,
+ VIRTIO_NET_F_MQ,
+ VIRTIO_NET_F_CTRL_MAC_ADDR
+ };
+ unsigned int features_for_guest_csum[] = {
+ VIRTIO_NET_F_GUEST_TSO4,
+ VIRTIO_NET_F_GUEST_TSO6,
+ VIRTIO_NET_F_GUEST_ECN,
+ VIRTIO_NET_F_GUEST_UFO,
+ };
+ unsigned int features_for_host_csum[] = {
+ VIRTIO_NET_F_HOST_TSO4,
+ VIRTIO_NET_F_HOST_TSO6,
+ VIRTIO_NET_F_HOST_ECN,
+ VIRTIO_NET_F_HOST_UFO,
+ };
+ int err;
+
+ err = virtnet_validate_features(dev, features_for_ctrl_vq,
+ ARRAY_SIZE(features_for_ctrl_vq),
+ VIRTIO_NET_F_CTRL_VQ);
+ if (err)
+ return err;
+
+ err = virtnet_validate_features(dev, features_for_guest_csum,
+ ARRAY_SIZE(features_for_guest_csum),
+ VIRTIO_NET_F_GUEST_CSUM);
+ if (err)
+ return err;
+
+ err = virtnet_validate_features(dev, features_for_host_csum,
+ ARRAY_SIZE(features_for_host_csum),
+ VIRTIO_NET_F_CSUM);
+ if (err)
+ return err;
+
+ if (virtio_has_feature(dev, VIRTIO_NET_F_GUEST_ECN) &&
+ (!virtio_has_feature(dev, VIRTIO_NET_F_GUEST_TSO4) ||
+ !virtio_has_feature(dev, VIRTIO_NET_F_GUEST_TSO6))) {
+ dev_err(&dev->dev,
+ "buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x or 0x%x was not",
+ VIRTIO_NET_F_GUEST_ECN,
+ VIRTIO_NET_F_GUEST_TSO4,
+ VIRTIO_NET_F_GUEST_TSO6);
+ return -EINVAL;
+ }
+
+ if (virtio_has_feature(dev, VIRTIO_NET_F_HOST_ECN) &&
+ (!virtio_has_feature(dev, VIRTIO_NET_F_HOST_TSO4) ||
+ !virtio_has_feature(dev, VIRTIO_NET_F_HOST_TSO6))) {
+ dev_err(&dev->dev,
+ "buggy hyperviser: feature 0x%x was advertised but its dependency 0x%x or 0x%x was not",
+ VIRTIO_NET_F_HOST_ECN,
+ VIRTIO_NET_F_HOST_TSO4,
+ VIRTIO_NET_F_HOST_TSO6);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int virtnet_probe(struct virtio_device *vdev)
{
int i, err;
@@ -1680,6 +1769,10 @@ static int virtnet_probe(struct virtio_device *vdev)
struct virtnet_info *vi;
u16 max_queue_pairs;
+ err = virtnet_check_features(vdev);
+ if (err)
+ return -EINVAL;
+
/* Find if host supports multiqueue virtio_net device */
err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
struct virtio_net_config,
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 0/4] igb: auxiliary PHC functions for the i210.
From: Richard Cochran @ 2014-11-19 6:31 UTC (permalink / raw)
To: Jeff Kirsher
Cc: netdev, David Miller, bruce.w.allan, Jacob Keller, John Ronciak,
Matthew Vick
In-Reply-To: <1416266118.2983.3.camel@jtkirshe-mobl.jf.intel.com>
On Mon, Nov 17, 2014 at 03:15:18PM -0800, Jeff Kirsher wrote:
> Thanks Richard, I have added your series of igb patches to my queue.
Jeff, please hold off on these. I found a bug in the PPS code. I will
re-submit with a fix.
Thanks,
Richard
^ permalink raw reply
* Re: [BNX2] A Netdev Watchdog with kernel stable 3.4
From: Rui Xiang @ 2014-11-19 6:28 UTC (permalink / raw)
To: Michael Chan; +Cc: netdev
In-Reply-To: <5469ED24.7010008@huawei.com>
ping...
On 2014/11/17 20:42, Rui Xiang wrote:
> Hi Michael,
>
> On a system that was running stable 3.4.87, I got the below stack.
> That was a NETDEV WATCHDOG. And we could also see watchdog timeouts with the
> BNX2. (After the stack, an oops occurred while running ifconfig. I think it
> would be related to this timeout.)
>
> Otherwises, the bnx2_dump_state and bnx2_dump_mcp_state have printed the states.
> Through these states info, can we got the real situation of NIC1.
> Or can we see what resulted the WATCHDOG, a bnx2 device fault or other reasons.
>
> Thanks.
>
>
> *The stack*:
>
> WARNING: at /usr/src/packages/BUILD/kernel-default-3.4.87/linux-3.4/net/sched/sch_generic.c:256 dev_watchdog+0x256/0x260()
> NETDEV WATCHDOG: NIC1 (bnx2): transmit queue 3 timed out
> Modules linked in: smb3_failover(O) smb2(O) smb(O) smb_manager(O) nfs(O) nfs_acl(O) nfsd(O) lockd(O) nal(O) auth_rpcgss(O)
> scsi_dh_hp_sw scsi_dh_alua scsi_dh_emc scsi_dh_rdac scsi_dh scsi_mod [last unloaded: ipmi_msghandler]
> Pid: 0, comm: swapper/0 Tainted: P W O 3.4.87-default #1
> Call Trace:
> <IRQ> [<ffffffff8103fcea>] warn_slowpath_common+0x7a/0xb0
> [<ffffffff8103fdc1>] warn_slowpath_fmt+0x41/0x50
> [<ffffffff81047749>] ? raise_softirq_irqoff+0x9/0x30
> [<ffffffff813ae0f6>] dev_watchdog+0x256/0x260
> [<ffffffff813adea0>] ? dev_deactivate_queue.constprop.30+0x70/0x70
> [<ffffffff8104edc7>] run_timer_softirq+0x147/0x340
> [<ffffffff810470d8>] __do_softirq+0xc8/0x1e0
> [<ffffffff8109250f>] ? tick_program_event+0x1f/0x30
> [<ffffffff81460a6c>] call_softirq+0x1c/0x30
> [<ffffffff8100417d>] do_softirq+0x9d/0xd0
> [<ffffffff810474a5>] irq_exit+0xb5/0xc0
> [<ffffffff81021b49>] smp_apic_timer_interrupt+0x69/0xa0
> [<ffffffff8146006f>] apic_timer_interrupt+0x6f/0x80
> <EOI> [<ffffffff81457bdd>] ? retint_restore_args+0x13/0x13
> [<ffffffff81360149>] ? poll_idle+0x49/0x90
> [<ffffffff8136011f>] ? poll_idle+0x1f/0x90
> [<ffffffff8135fcc9>] cpuidle_enter+0x19/0x20
> [<ffffffff813602f2>] cpuidle_idle_call+0xa2/0x250
> [<ffffffff8100b08f>] cpu_idle+0x6f/0xe0
> [<ffffffff81915960>] ? rawsock_init+0x12/0x12
> [<ffffffff814331c9>] rest_init+0x6d/0x74
> [<ffffffff818d3be5>] start_kernel+0x3a2/0x3af
> [<ffffffff818d3642>] ? repair_env_string+0x5e/0x5e
> [<ffffffff818d332a>] x86_64_start_reservations+0x131/0x135
> [<ffffffff818d342e>] x86_64_start_kernel+0x100/0x10f
> ---[ end trace 497e24e681e0c02d ]---
> bnx2 0000:05:00.1: NIC1: DEBUG: intr_sem[0] PCI_CMD[00100002]
> bnx2 0000:05:00.1: NIC1: DEBUG: PCI_PM[19002008] PCI_MISC_CFG[92000088]
> bnx2 0000:05:00.1: NIC1: DEBUG: EMAC_TX_STATUS[00000008] EMAC_RX_STATUS[00000000]
> bnx2 0000:05:00.1: NIC1: DEBUG: RPM_MGMT_PKT_CTRL[40000088]
> bnx2 0000:05:00.1: NIC1: DEBUG: HC_STATS_INTERRUPT_STATUS[01ff0000]
> bnx2 0000:05:00.1: NIC1: DEBUG: PBA[00000000]
> bnx2 0000:05:00.1: NIC1: <--- start MCP states dump --->
> bnx2 0000:05:00.1: NIC1: DEBUG: MCP_STATE_P0[0003e10e] MCP_STATE_P1[0003e10e]
> bnx2 0000:05:00.1: NIC1: DEBUG: MCP mode[0000b800] state[80008000] evt_mask[00000500]
> bnx2 0000:05:00.1: NIC1: DEBUG: pc[08008f60] pc[0800d21c] instr[00051080]
> bnx2 0000:05:00.1: NIC1: DEBUG: shmem states:
> bnx2 0000:05:00.1: NIC1: DEBUG: drv_mb[01030003] fw_mb[00000003] link_status[0000006f] drv_pulse_mb[0000073d]
> bnx2 0000:05:00.1: NIC1: DEBUG: dev_info_signature[44564907] reset_type[01005254] condition[0003e10e]
> bnx2 0000:05:00.1: NIC1: DEBUG: 000003cc: 00000000 00000000 00000000 00000000
> bnx2 0000:05:00.1: NIC1: DEBUG: 000003dc: 00000000 00000000 00000000 00000000
> bnx2 0000:05:00.1: NIC1: DEBUG: 000003ec: 00000000 00000000 00000000 00000000
> bnx2 0000:05:00.1: NIC1: DEBUG: 0x3fc[00000000]
> bnx2 0000:05:00.1: NIC1: <--- end MCP states dump --->
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox