* [PATCH 0/3] netdev/phy/of: Improve 10G Ethernet PHY support.
From: David Daney @ 2011-10-12 18:06 UTC (permalink / raw)
To: devicetree-discuss, grant.likely, linux-kernel, netdev, davem
Cc: afleming, David Daney
The PHY driver core needs a couple of adjustments to be able to
support 10G PHYs. The main issue being that they use a different MDIO
bus protocol (IEEE802.3 clause 45).
The changes are not large:
1) The addr argument to get_phy_id() and get_phy_device() can be
flagged to indicate clause 45 addressing.
2) The device tree helper of_mdiobus_register() uses a new
"compatible" value to hook up the 10G phys.
3) A driver for the BCM8706 which makes use of it all.
David Daney (3):
netdev/phy: Handle IEEE802.3 clause 45 Ethernet PHYs
netdev/phy/of: Handle IEEE802.3 clause 45 Ethernet PHYs in
of_mdiobus_register()
netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY
.../devicetree/bindings/net/broadcom-bcm8706.txt | 28 +++
Documentation/devicetree/bindings/net/phy.txt | 12 +-
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/bcm8706.c | 212 ++++++++++++++++++++
drivers/net/phy/phy_device.c | 25 ++-
drivers/of/of_mdio.c | 4 +
include/linux/brcmphy.h | 1 +
include/linux/phy.h | 3 +
9 files changed, 287 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
create mode 100644 drivers/net/phy/bcm8706.c
--
1.7.2.3
^ permalink raw reply
* [PATCH 1/3] netdev/phy: Handle IEEE802.3 clause 45 Ethernet PHYs
From: David Daney @ 2011-10-12 18:06 UTC (permalink / raw)
To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: afleming-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1318442783-29058-1-git-send-email-david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
The IEEE802.3 clause 45 MDIO bus protocol allows for directly
addressing PHY registers using a 21 bit address, and is used by many
10G Ethernet PHYS. Already existing is the ability of MDIO bus
drivers to use clause 45, with the MII_ADDR_C45 flag. Here we add
some support in the PHY and device tree infrastructure to use these
PHYs.
Normally the MII_ADDR_C45 flag is ORed with the register address to
indicate a clause 45 transaction. Here we also use this flag in the
*device* address passed to get_phy_id() and get_phy_device() to
indicate that probing should be done with clause 45 transactions. If
a PHY is successfully probed with MII_ADDR_C45, the new struct
phy_device is_c45 flag is set for the PHY.
Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
drivers/net/phy/phy_device.c | 25 ++++++++++++++++++++++---
include/linux/phy.h | 3 +++
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 83a5a5a..7e4d61b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -171,6 +171,8 @@ static struct phy_device* phy_device_create(struct mii_bus *bus,
dev->autoneg = AUTONEG_ENABLE;
+ dev->is_c45 = (addr & MII_ADDR_C45) != 0;
+ addr &= ~MII_ADDR_C45;
dev->addr = addr;
dev->phy_id = phy_id;
dev->bus = bus;
@@ -205,15 +207,24 @@ static struct phy_device* phy_device_create(struct mii_bus *bus,
* @phy_id: where to store the ID retrieved.
*
* Description: Reads the ID registers of the PHY at @addr on the
- * @bus, stores it in @phy_id and returns zero on success.
+ * @bus, stores it in @phy_id and returns zero on success. If the
+ * @addr has been ORed with MII_ADDR_C45, mdio clause 45 data
+ * transfer is used to read ID from the PHY device, otherwise the
+ * standard protocol (clause 22) is used.
*/
int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id)
{
int phy_reg;
+ u32 c45_reg_base = 0;
+ if (addr & MII_ADDR_C45) {
+ addr &= ~MII_ADDR_C45;
+ /* Access the PHY's PHY XS registers with C45 mode. */
+ c45_reg_base = MII_ADDR_C45 | 0x40000;
+ }
/* Grab the bits from PHYIR1, and put them
* in the upper half */
- phy_reg = mdiobus_read(bus, addr, MII_PHYSID1);
+ phy_reg = mdiobus_read(bus, addr, c45_reg_base | MII_PHYSID1);
if (phy_reg < 0)
return -EIO;
@@ -221,7 +232,7 @@ int get_phy_id(struct mii_bus *bus, int addr, u32 *phy_id)
*phy_id = (phy_reg & 0xffff) << 16;
/* Grab the bits from PHYIR2, and put them in the lower half */
- phy_reg = mdiobus_read(bus, addr, MII_PHYSID2);
+ phy_reg = mdiobus_read(bus, addr, c45_reg_base | MII_PHYSID2);
if (phy_reg < 0)
return -EIO;
@@ -239,6 +250,9 @@ EXPORT_SYMBOL(get_phy_id);
*
* Description: Reads the ID registers of the PHY at @addr on the
* @bus, then allocates and returns the phy_device to represent it.
+ * If the @addr has been ORed with MII_ADDR_C45, mdio clause 45 data
+ * transfer is used to read the PHY device, otherwise the standard
+ * protocol (clause 22) is used.
*/
struct phy_device * get_phy_device(struct mii_bus *bus, int addr)
{
@@ -447,6 +461,11 @@ static int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
/* Assume that if there is no driver, that it doesn't
* exist, and we should use the genphy driver. */
if (NULL == d->driver) {
+ if (phydev->is_c45) {
+ pr_err("No driver for phy %x\n", phydev->phy_id);
+ return -ENODEV;
+ }
+
d->driver = &genphy_driver.driver;
err = d->driver->probe(d);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e4c3844..0a25e2c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -246,6 +246,7 @@ struct sk_buff;
* phy_id: UID for this device found during discovery
* state: state of the PHY for management purposes
* dev_flags: Device-specific flags used by the PHY driver.
+ * is_c45: Set to 1 if this phy uses clause 45 addressing.
* addr: Bus address of PHY
* link_timeout: The number of timer firings to wait before the
* giving up on the current attempt at acquiring a link
@@ -283,6 +284,8 @@ struct phy_device {
u32 dev_flags;
+ unsigned int is_c45:1;
+
phy_interface_t interface;
/* Bus address of the PHY (0-31) */
--
1.7.2.3
^ permalink raw reply related
* [PATCH 2/3] netdev/phy/of: Handle IEEE802.3 clause 45 Ethernet PHYs in of_mdiobus_register()
From: David Daney @ 2011-10-12 18:06 UTC (permalink / raw)
To: devicetree-discuss, grant.likely, linux-kernel, netdev, davem
Cc: afleming, David Daney
In-Reply-To: <1318442783-29058-1-git-send-email-david.daney@cavium.com>
Define two new "compatible" values for Ethernet
PHYs. "ethernet-phy-ieee802.3-c22" and "ethernet-phy-ieee802.3-c45"
are used to indicate a PHY uses the corresponding protocol.
If a PHY is "compatible" with "ethernet-phy-ieee802.3-c45", we
indicate this so that get_phy_device() can properly probe the device.
Signed-off-by: David Daney <david.daney@cavium.com>
---
Documentation/devicetree/bindings/net/phy.txt | 12 +++++++++++-
drivers/of/of_mdio.c | 4 ++++
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index bb8c742..de9cb32 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -16,8 +16,18 @@ Required properties:
Example:
+Optional Properties:
+
+- compatible: Compatible list, may contain "ethernet-phy-ieee802.3-c22" or
+ "ethernet-phy-ieee802.3-c45" for PHYs that implement
+ IEEE802.3 clause 22 or IEEE802.3 clause 45
+ specifications. If neither of these are specified, the
+ default is to assume clause 22. The compatible list may
+ also contain other elements.
+
ethernet-phy@0 {
- linux,phandle = <2452000>
+ compatible = "ethernet-phy-ieee802.3-c22";
+ linux,phandle = <2452000>;
interrupt-parent = <40000>;
interrupts = <35 1>;
reg = <0>;
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 7c28e8c..f837a7f 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -79,6 +79,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
mdio->irq[addr] = PHY_POLL;
}
+ if (of_device_is_compatible(child,
+ "ethernet-phy-ieee802.3-c45"))
+ addr |= MII_ADDR_C45;
+
phy = get_phy_device(mdio, addr);
if (!phy || IS_ERR(phy)) {
dev_err(&mdio->dev, "error probing PHY at address %i\n",
--
1.7.2.3
^ permalink raw reply related
* [PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY
From: David Daney @ 2011-10-12 18:06 UTC (permalink / raw)
To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: afleming-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1318442783-29058-1-git-send-email-david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
Add a driver and PHY_ID number for said device. This is a 10Gig PHY
which uses MII_ADDR_C45 addressing, it is always 10G full duplex, so
there is no autonegotiation. All we do is report link state and send
interrupts when it changes.
If the PHY has a device tree of_node associated with it, the
"broadcom,c45-reg-init" property is used to supply register
initialization values when config_init() is called.
Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
---
.../devicetree/bindings/net/broadcom-bcm8706.txt | 28 +++
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/bcm8706.c | 212 ++++++++++++++++++++
include/linux/brcmphy.h | 1 +
5 files changed, 247 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
create mode 100644 drivers/net/phy/bcm8706.c
diff --git a/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
new file mode 100644
index 0000000..d58bea9
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
@@ -0,0 +1,28 @@
+The Broadcom BCM8706 is a 10G Ethernet PHY. It has these bindings in
+addition to the standard PHY bindings.
+
+Compatible: Should contain "broadcom,bcm8706" and
+ "ethernet-phy-ieee802.3-c45"
+
+Optional Properties:
+
+- broadcom,c45-reg-init : one of more sets of 4 cells. The first cell
+ is the device type, the second a register address, the third cell
+ contains a mask to be ANDed with the existing register value, and
+ the fourth cell is ORed with he result to yield the new register
+ value.
+
+Example:
+
+ ethernet-phy@5 {
+ reg = <5>;
+ compatible = "broadcom,bcm8706", "ethernet-phy-ieee802.3-c45";
+ interrupt-parent = <&gpio>;
+ interrupts = <12 8>; /* Pin 12, active low */
+ /*
+ * Set PMD Digital Control Register for
+ * GPIO[1] Tx/Rx
+ * GPIO[0] R64 Sync Acquired
+ */
+ broadcom,c45-reg-init = <1 0xc808 0xff8f 0x70>;
+ };
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 59b3b17..a99885f 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -62,6 +62,11 @@ config BCM63XX_PHY
---help---
Currently supports the 6348 and 6358 PHYs.
+config BCM8706_PHY
+ tristate "Driver for Broadcom BCM8706 10G Ethernet PHY"
+ help
+ Currently supports only the BCM8706 PHY.
+
config ICPLUS_PHY
tristate "Drivers for ICPlus PHYs"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index d1a1927..ec46398 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SMSC_PHY) += smsc.o
obj-$(CONFIG_VITESSE_PHY) += vitesse.o
obj-$(CONFIG_BROADCOM_PHY) += broadcom.o
obj-$(CONFIG_BCM63XX_PHY) += bcm63xx.o
+obj-$(CONFIG_BCM8706_PHY) += bcm8706.o
obj-$(CONFIG_ICPLUS_PHY) += icplus.o
obj-$(CONFIG_REALTEK_PHY) += realtek.o
obj-$(CONFIG_LSI_ET1011C_PHY) += et1011c.o
diff --git a/drivers/net/phy/bcm8706.c b/drivers/net/phy/bcm8706.c
new file mode 100644
index 0000000..3a23e04
--- /dev/null
+++ b/drivers/net/phy/bcm8706.c
@@ -0,0 +1,212 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2011 Cavium, Inc.
+ */
+
+#include <linux/module.h>
+#include <linux/phy.h>
+#include <linux/brcmphy.h>
+#include <linux/of.h>
+
+#define BCM8706_PMD_RX_SIGNAL_DETECT (MII_ADDR_C45 | 0x1000a)
+#define BCM8706_10GBASER_PCS_STATUS (MII_ADDR_C45 | 0x30020)
+#define BCM8706_XGXS_LANE_STATUS (MII_ADDR_C45 | 0x40018)
+
+#define BCM8706_LASI_CONTROL (MII_ADDR_C45 | 0x39002)
+#define BCM8706_LASI_STATUS (MII_ADDR_C45 | 0x39005)
+
+#ifdef CONFIG_OF_MDIO
+/*
+ * Set and/or override some configuration registers based on the
+ * broadcom,c45-reg-init property stored in the of_node for the phydev.
+ *
+ * broadcom,c45-reg-init = <devid reg mask value>,...;
+ *
+ * There may be one or more sets of <devid reg mask value>:
+ *
+ * devid: which sub-device to use.
+ * reg: the register.
+ * mask: if non-zero, ANDed with existing register value.
+ * value: ORed with the masked value and written to the regiser.
+ *
+ */
+static int bcm8706_of_reg_init(struct phy_device *phydev)
+{
+ const __be32 *paddr;
+ int len, i, ret;
+
+ if (!phydev->dev.of_node)
+ return 0;
+
+ paddr = of_get_property(phydev->dev.of_node,
+ "broadcom,c45-reg-init", &len);
+ if (!paddr || len < (4 * sizeof(*paddr)))
+ return 0;
+
+ ret = 0;
+ len /= sizeof(*paddr);
+ for (i = 0; i < len - 3; i += 4) {
+ u16 devid = be32_to_cpup(paddr + i);
+ u16 reg = be32_to_cpup(paddr + i + 1);
+ u16 mask = be32_to_cpup(paddr + i + 2);
+ u16 val_bits = be32_to_cpup(paddr + i + 3);
+ int val;
+ u32 regnum = MII_ADDR_C45 | (devid << 16) | reg;
+ val = 0;
+ if (mask) {
+ val = phy_read(phydev, regnum);
+ if (val < 0) {
+ ret = val;
+ goto err;
+ }
+ val &= mask;
+ }
+ val |= val_bits;
+
+ ret = phy_write(phydev, regnum, val);
+ if (ret < 0)
+ goto err;
+
+ }
+err:
+ return ret;
+}
+#else
+static int bcm8706_of_reg_init(struct phy_device *phydev)
+{
+ return 0;
+}
+#endif /* CONFIG_OF_MDIO */
+
+static int bcm8706_config_init(struct phy_device *phydev)
+{
+ phydev->supported = SUPPORTED_10000baseR_FEC;
+ phydev->advertising = ADVERTISED_10000baseR_FEC;
+ phydev->state = PHY_NOLINK;
+
+ bcm8706_of_reg_init(phydev);
+
+ return 0;
+}
+
+static int bcm8706_config_aneg(struct phy_device *phydev)
+{
+ return -EINVAL;
+}
+
+static int bcm8706_read_status(struct phy_device *phydev)
+{
+ int rx_signal_detect;
+ int pcs_status;
+ int xgxs_lane_status;
+
+ rx_signal_detect = phy_read(phydev, BCM8706_PMD_RX_SIGNAL_DETECT);
+ if (rx_signal_detect < 0)
+ return rx_signal_detect;
+
+ if ((rx_signal_detect & 1) == 0)
+ goto no_link;
+
+ pcs_status = phy_read(phydev, BCM8706_10GBASER_PCS_STATUS);
+ if (pcs_status < 0)
+ return pcs_status;
+
+ if ((pcs_status & 1) == 0)
+ goto no_link;
+
+ xgxs_lane_status = phy_read(phydev, BCM8706_XGXS_LANE_STATUS);
+ if (xgxs_lane_status < 0)
+ return xgxs_lane_status;
+
+ if ((xgxs_lane_status & 0x1000) == 0)
+ goto no_link;
+
+ phydev->speed = 10000;
+ phydev->link = 1;
+ phydev->duplex = 1;
+ return 0;
+
+no_link:
+ phydev->link = 0;
+ return 0;
+}
+
+static int bcm8706_config_intr(struct phy_device *phydev)
+{
+ int reg, err;
+
+ reg = phy_read(phydev, BCM8706_LASI_CONTROL);
+
+ if (reg < 0)
+ return reg;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ reg |= 1;
+ else
+ reg &= ~1;
+
+ err = phy_write(phydev, BCM8706_LASI_CONTROL, reg);
+ return err;
+}
+
+static int bcm8706_did_interrupt(struct phy_device *phydev)
+{
+ int reg;
+
+ reg = phy_read(phydev, BCM8706_LASI_STATUS);
+
+ if (reg < 0) {
+ dev_err(&phydev->dev,
+ "Error: Read of BCM8706_LASI_STATUS failed: %d\n", reg);
+ return 0;
+ }
+ return (reg & 1) != 0;
+}
+
+static int bcm8706_ack_interrupt(struct phy_device *phydev)
+{
+ /* Reading the LASI status clears it. */
+ bcm8706_did_interrupt(phydev);
+ return 0;
+}
+
+
+static struct phy_driver bcm8706_driver = {
+ .phy_id = PHY_ID_BCM8706,
+ .phy_id_mask = 0xffffffff,
+ .name = "Broadcom BCM8706",
+ .flags = PHY_HAS_INTERRUPT,
+ .config_init = bcm8706_config_init,
+ .config_aneg = bcm8706_config_aneg,
+ .read_status = bcm8706_read_status,
+ .ack_interrupt = bcm8706_ack_interrupt,
+ .config_intr = bcm8706_config_intr,
+ .did_interrupt = bcm8706_did_interrupt,
+ .driver = { .owner = THIS_MODULE },
+};
+
+static int __init bcm8706_init(void)
+{
+ int ret;
+
+ ret = phy_driver_register(&bcm8706_driver);
+
+ return ret;
+}
+module_init(bcm8706_init);
+
+static void __exit bcm8706_exit(void)
+{
+ phy_driver_unregister(&bcm8706_driver);
+}
+module_exit(bcm8706_exit);
+
+static struct mdio_device_id __maybe_unused bcm8706_tbl[] = {
+ { PHY_ID_BCM8706, 0xffffffff },
+ { }
+};
+
+MODULE_DEVICE_TABLE(mdio, bcm8706_tbl);
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index b840a49..e06a56a 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -9,6 +9,7 @@
#define PHY_ID_BCM5464 0x002060b0
#define PHY_ID_BCM5461 0x002060c0
#define PHY_ID_BCM57780 0x03625d90
+#define PHY_ID_BCM8706 0x0143bdc1
#define PHY_BCM_OUI_MASK 0xfffffc00
#define PHY_BCM_OUI_1 0x00206000
--
1.7.2.3
^ permalink raw reply related
* [PATCH 1/1] net: hold sock reference while processing tx timestamps
From: Richard Cochran @ 2011-10-12 18:36 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Johannes Berg, stable
In-Reply-To: <1318007501.3988.20.camel@jlt3.sipsolutions.net>
The pair of functions,
* skb_clone_tx_timestamp()
* skb_complete_tx_timestamp()
were designed to allow timestamping in PHY devices. The first
function, called during the MAC driver's hard_xmit method, identifies
PTP protocol packets, clones them, and gives them to the PHY device
driver. The PHY driver may hold onto the packet and deliver it at a
later time using the second function, which adds the packet to the
socket's error queue.
As pointed out by Johannes, nothing prevents the socket from
disappearing while the cloned packet is sitting in the PHY driver
awaiting a timestamp. This patch fixes the issue by taking a reference
on the socket for each such packet. In addition, the comments
regarding the usage of these function are expanded to highlight the
rule that PHY drivers must use skb_complete_tx_timestamp() to release
the packet, in order to release the socket reference, too.
These functions first appeared in v2.6.36.
Reported-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
Cc: <stable@kernel.org>
---
include/linux/phy.h | 2 +-
include/linux/skbuff.h | 7 ++++++-
net/core/timestamping.c | 7 ++++++-
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 54fc413..79f337c 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -420,7 +420,7 @@ struct phy_driver {
/*
* Requests a Tx timestamp for 'skb'. The phy driver promises
- * to deliver it to the socket's error queue as soon as a
+ * to deliver it using skb_complete_tx_timestamp() as soon as a
* timestamp becomes available. One of the PTP_CLASS_ values
* is passed in 'type'.
*/
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 8bd383c..0f96646 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2020,8 +2020,13 @@ static inline bool skb_defer_rx_timestamp(struct sk_buff *skb)
/**
* skb_complete_tx_timestamp() - deliver cloned skb with tx timestamps
*
+ * PHY drivers may accept clones of transmitted packets for
+ * timestamping via their phy_driver.txtstamp method. These drivers
+ * must call this function to return the skb back to the stack, with
+ * or without a timestamp.
+ *
* @skb: clone of the the original outgoing packet
- * @hwtstamps: hardware time stamps
+ * @hwtstamps: hardware time stamps, may be NULL if not available
*
*/
void skb_complete_tx_timestamp(struct sk_buff *skb,
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index 98a5264..29e59d6 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -60,6 +60,7 @@ void skb_clone_tx_timestamp(struct sk_buff *skb)
clone = skb_clone(skb, GFP_ATOMIC);
if (!clone)
return;
+ sock_hold(sk);
clone->sk = sk;
phydev->drv->txtstamp(phydev, clone, type);
}
@@ -77,8 +78,11 @@ void skb_complete_tx_timestamp(struct sk_buff *skb,
struct sock_exterr_skb *serr;
int err;
- if (!hwtstamps)
+ if (!hwtstamps) {
+ sock_put(sk);
+ kfree_skb(skb);
return;
+ }
*skb_hwtstamps(skb) = *hwtstamps;
serr = SKB_EXT_ERR(skb);
@@ -87,6 +91,7 @@ void skb_complete_tx_timestamp(struct sk_buff *skb,
serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;
skb->sk = NULL;
err = sock_queue_err_skb(sk, skb);
+ sock_put(sk);
if (err)
kfree_skb(skb);
}
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH v3] net-netlink: Add a new attribute to expose TOS values via netlink
From: MuraliRaja Muniraju @ 2011-10-12 19:00 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, linux-kernel, netdev
In-Reply-To: <1318393869.3686.26.camel@edumazet-laptop>
Hi Eric,
I am planning to not add code for the TIME_WAIT case as the tos
values are zero as they are not saved. Please let me if this is OK.
Thanks,
Murali
On Tue, Oct 11, 2011 at 9:31 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 11 octobre 2011 à 21:16 -0700, MuraliRaja Muniraju a écrit :
>> Eric,
>> I think it would be useful to expose the values this way.
>> I shall make the changes to add TCLASS for ipv6 in the description.
>>
>> Regarding the TIME_WAIT state, I think it would be better of not
>> exposing the values because there would hardly be anything to transmit
>> during this time.
>>
>
> My remark had nothing to do with your patch actually :
>
> We dont store TOS/TCLASS on TIME_WAIT sockets, so you cannot report
> value in inet_diag.
>
> But we do send packets on behalf of TIME_WAIT sockets ;)
>
> Think about ACK messages. They probably are sent with a 0 TOS/TCLASS
> field... I am not sure its a problem or not.
>
>
>
>
--
Thanks,
Murali
^ permalink raw reply
* [PATCH v4] net-netlink: Add a new attribute to expose TOS values via netlink
From: Muraliraja Muniraju @ 2011-10-12 19:00 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy <kabe
Cc: linux-kernel, netdev, Murali Raja
In-Reply-To: <20111012085235.3b78729b@nehalam.linuxnetplumber.net>
From: Murali Raja <muralira@google.com>
This patch exposes the tos value for the TCP sockets when the TOS flag
is requested in the ext_flags for the inet_diag request. This would mainly be
used to expose TOS values for both for TCP and UDP sockets. Currently it is
supported for TCP. When netlink support for UDP would be added the support
to expose the TOS values would alse be done. For IPV4 tos value is exposed
and for IPV6 tclass value is exposed.
Signed-off-by: Murali Raja <muralira@google.com>
---
Changelog since v3:
- Removed the tos structure from the inet_diag.h
Changelog since v2:
- Added support for IPv6 and used better API.
Changelog since v3:
- Removed reserved fields.
include/linux/inet_diag.h | 3 ++-
net/ipv4/inet_diag.c | 5 +++++
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/include/linux/inet_diag.h b/include/linux/inet_diag.h
index bc8c490..80b480c 100644
--- a/include/linux/inet_diag.h
+++ b/include/linux/inet_diag.h
@@ -97,9 +97,10 @@ enum {
INET_DIAG_INFO,
INET_DIAG_VEGASINFO,
INET_DIAG_CONG,
+ INET_DIAG_TOS,
};
-#define INET_DIAG_MAX INET_DIAG_CONG
+#define INET_DIAG_MAX INET_DIAG_TOS
/* INET_DIAG_MEM */
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 389a2e6..f5e2bda 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -108,6 +108,9 @@ static int inet_csk_diag_fill(struct sock *sk,
icsk->icsk_ca_ops->name);
}
+ if ((ext & (1 << (INET_DIAG_TOS - 1))) && (sk->sk_family != AF_INET6))
+ RTA_PUT_U8(skb, INET_DIAG_TOS, inet->tos);
+
r->idiag_family = sk->sk_family;
r->idiag_state = sk->sk_state;
r->idiag_timer = 0;
@@ -130,6 +133,8 @@ static int inet_csk_diag_fill(struct sock *sk,
&np->rcv_saddr);
ipv6_addr_copy((struct in6_addr *)r->id.idiag_dst,
&np->daddr);
+ if (ext & (1 << (INET_DIAG_TOS - 1)))
+ RTA_PUT_U8(skb, INET_DIAG_TOS, np->tclass);
}
#endif
--
1.7.3.1
^ permalink raw reply related
* Re: [PATCH v4] net-netlink: Add a new attribute to expose TOS values via netlink
From: Stephen Hemminger @ 2011-10-12 19:08 UTC (permalink / raw)
To: Muraliraja Muniraju
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, linux-kernel, netdev
In-Reply-To: <1318446035-10267-1-git-send-email-muralira@google.com>
On Wed, 12 Oct 2011 12:00:35 -0700
Muraliraja Muniraju <muralira@google.com> wrote:
> From: Murali Raja <muralira@google.com>
>
> This patch exposes the tos value for the TCP sockets when the TOS flag
> is requested in the ext_flags for the inet_diag request. This would mainly be
> used to expose TOS values for both for TCP and UDP sockets. Currently it is
> supported for TCP. When netlink support for UDP would be added the support
> to expose the TOS values would alse be done. For IPV4 tos value is exposed
> and for IPV6 tclass value is exposed.
>
> Signed-off-by: Murali Raja <muralira@google.com>
> ---
> Changelog since v3:
> - Removed the tos structure from the inet_diag.h
>
> Changelog since v2:
> - Added support for IPv6 and used better API.
>
> Changelog since v3:
> - Removed reserved fields.
After this is accepted; it is trivial to add support to ss command.
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
^ permalink raw reply
* Re: [PATCH v4] net-netlink: Add a new attribute to expose TOS values via netlink
From: Eric Dumazet @ 2011-10-12 19:15 UTC (permalink / raw)
To: Muraliraja Muniraju
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, linux-kernel, netdev
In-Reply-To: <1318446035-10267-1-git-send-email-muralira@google.com>
Le mercredi 12 octobre 2011 à 12:00 -0700, Muraliraja Muniraju a écrit :
> From: Murali Raja <muralira@google.com>
>
> This patch exposes the tos value for the TCP sockets when the TOS flag
> is requested in the ext_flags for the inet_diag request. This would mainly be
> used to expose TOS values for both for TCP and UDP sockets. Currently it is
> supported for TCP. When netlink support for UDP would be added the support
> to expose the TOS values would alse be done. For IPV4 tos value is exposed
> and for IPV6 tclass value is exposed.
>
> Signed-off-by: Murali Raja <muralira@google.com>
> ---
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/1] net: hold sock reference while processing tx timestamps
From: Eric Dumazet @ 2011-10-12 19:25 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev, David Miller, Johannes Berg, stable
In-Reply-To: <56185ca8a7dc0223031ca0f0996302cac1b497eb.1318444117.git.richard.cochran@omicron.at>
Le mercredi 12 octobre 2011 à 20:36 +0200, Richard Cochran a écrit :
> The pair of functions,
>
> * skb_clone_tx_timestamp()
> * skb_complete_tx_timestamp()
>
> were designed to allow timestamping in PHY devices. The first
> function, called during the MAC driver's hard_xmit method, identifies
> PTP protocol packets, clones them, and gives them to the PHY device
> driver. The PHY driver may hold onto the packet and deliver it at a
> later time using the second function, which adds the packet to the
> socket's error queue.
>
> As pointed out by Johannes, nothing prevents the socket from
> disappearing while the cloned packet is sitting in the PHY driver
> awaiting a timestamp. This patch fixes the issue by taking a reference
> on the socket for each such packet. In addition, the comments
> regarding the usage of these function are expanded to highlight the
> rule that PHY drivers must use skb_complete_tx_timestamp() to release
> the packet, in order to release the socket reference, too.
>
> These functions first appeared in v2.6.36.
>
> Reported-by: Johannes Berg <johannes@sipsolutions.net>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> Cc: <stable@kernel.org>
> ---
Seems fine to me, thanks !
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/1] net: hold sock reference while processing tx timestamps
From: Johannes Berg @ 2011-10-12 19:27 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev, David Miller, Eric Dumazet
In-Reply-To: <56185ca8a7dc0223031ca0f0996302cac1b497eb.1318444117.git.richard.cochran@omicron.at>
On Wed, 2011-10-12 at 20:36 +0200, Richard Cochran wrote:
> The pair of functions,
>
> * skb_clone_tx_timestamp()
> * skb_complete_tx_timestamp()
>
> were designed to allow timestamping in PHY devices. The first
> function, called during the MAC driver's hard_xmit method, identifies
> PTP protocol packets, clones them, and gives them to the PHY device
> driver. The PHY driver may hold onto the packet and deliver it at a
> later time using the second function, which adds the packet to the
> socket's error queue.
>
> As pointed out by Johannes, nothing prevents the socket from
> disappearing while the cloned packet is sitting in the PHY driver
> awaiting a timestamp. This patch fixes the issue by taking a reference
> on the socket for each such packet. In addition, the comments
> regarding the usage of these function are expanded to highlight the
> rule that PHY drivers must use skb_complete_tx_timestamp() to release
> the packet, in order to release the socket reference, too.
This still needs a fix to the PHY driver right? It has a case that can
kfree_skb() the skb instead of passing it back to
complete_tx_timestamp().
> - if (!hwtstamps)
> + if (!hwtstamps) {
> + sock_put(sk);
> + kfree_skb(skb);
> return;
> + }
Is that right w/o skb->sk = NULL?
The other thing I was wondering -- what if we just set truesize to 1 (we
don't have any truesize checks) and account the skb to the socket
normally. Not really a good way either though.
FWIW I just decided to do it the other way around in mac80211 -- keep
the original SKB that's charged to the socket for the error queue, and
use a clone to actually do the TX.
johannes
^ permalink raw reply
* Re: [PATCH 1/1] net: hold sock reference while processing tx timestamps
From: Eric Dumazet @ 2011-10-12 19:52 UTC (permalink / raw)
To: Johannes Berg; +Cc: Richard Cochran, netdev, David Miller
In-Reply-To: <1318447673.3933.47.camel@jlt3.sipsolutions.net>
Le mercredi 12 octobre 2011 à 21:27 +0200, Johannes Berg a écrit :
> On Wed, 2011-10-12 at 20:36 +0200, Richard Cochran wrote:
> > The pair of functions,
> >
> > * skb_clone_tx_timestamp()
> > * skb_complete_tx_timestamp()
> >
> > were designed to allow timestamping in PHY devices. The first
> > function, called during the MAC driver's hard_xmit method, identifies
> > PTP protocol packets, clones them, and gives them to the PHY device
> > driver. The PHY driver may hold onto the packet and deliver it at a
> > later time using the second function, which adds the packet to the
> > socket's error queue.
> >
> > As pointed out by Johannes, nothing prevents the socket from
> > disappearing while the cloned packet is sitting in the PHY driver
> > awaiting a timestamp. This patch fixes the issue by taking a reference
> > on the socket for each such packet. In addition, the comments
> > regarding the usage of these function are expanded to highlight the
> > rule that PHY drivers must use skb_complete_tx_timestamp() to release
> > the packet, in order to release the socket reference, too.
>
> This still needs a fix to the PHY driver right? It has a case that can
> kfree_skb() the skb instead of passing it back to
> complete_tx_timestamp().
>
> > - if (!hwtstamps)
> > + if (!hwtstamps) {
> > + sock_put(sk);
> > + kfree_skb(skb);
> > return;
> > + }
>
> Is that right w/o skb->sk = NULL?
>
>
> The other thing I was wondering -- what if we just set truesize to 1 (we
> don't have any truesize checks) and account the skb to the socket
> normally. Not really a good way either though.
>
Changing truesize is not allowed here see below.
> FWIW I just decided to do it the other way around in mac80211 -- keep
> the original SKB that's charged to the socket for the error queue, and
> use a clone to actually do the TX.
Hmm, please take a look at IFF_SKB_TX_SHARING stuff, added in commit
d88733150 (net: add IFF_SKB_TX_SHARED flag to priv_flags)
^ permalink raw reply
* Re: [net-next 2/5] stmmac: allow mtu bigger than 1500 in case of normal desc (V2).
From: Eric Dumazet @ 2011-10-12 19:58 UTC (permalink / raw)
To: Giuseppe CAVALLARO; +Cc: netdev, davem, Deepak SIKRI
In-Reply-To: <1318426688-9419-3-git-send-email-peppe.cavallaro@st.com>
Le mercredi 12 octobre 2011 à 15:38 +0200, Giuseppe CAVALLARO a écrit :
> This patch allows to set the mtu bigger than 1500
> in case of normal descriptors.
> This is helping some SPEAr customers.
>
> Signed-off-by: Deepak SIKRI <deepak.sikri@st.com>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index ba7af2c..de3e536 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1357,17 +1357,17 @@ static void stmmac_set_rx_mode(struct net_device *dev)
> static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
> {
> struct stmmac_priv *priv = netdev_priv(dev);
> - int max_mtu;
> + int max_mtu = ETH_DATA_LEN;
Why are you setting max_mtu to ETH_DATA_LEN here ?
>
> if (netif_running(dev)) {
> pr_err("%s: must be stopped to change its MTU\n", dev->name);
> return -EBUSY;
> }
>
> - if (priv->plat->has_gmac)
> + if (priv->plat->enh_desc)
> max_mtu = JUMBO_LEN;
> else
> - max_mtu = ETH_DATA_LEN;
> + max_mtu = BUF_SIZE_4KiB;
Since later you init to completely different values...
^ permalink raw reply
* Re: [net-next 10/12] igb: move DMA Coalescing feature code into separate function.
From: David Miller @ 2011-10-12 20:48 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: jacob.r.sowles, netdev, gospo, sassmann
In-Reply-To: <1318390708-12232-11-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 11 Oct 2011 20:38:26 -0700
> From: Jacob Sowles <jacob.r.sowles@intel.com>
>
> DMA Coalescing code needs to be executed during reset, if enabled. Moved
> code to separate function and added a call to it in igb_reset.
>
> Signed-off-by: Jacob Sowles <jacob.r.sowles@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Something is wrong here, this patch just adds the new function but no
actual callers.
^ permalink raw reply
* Re: [PATCH 1/4] ipv4: Fix pmtu propagating
From: David Miller @ 2011-10-12 21:02 UTC (permalink / raw)
To: steffen.klassert; +Cc: netdev
In-Reply-To: <20111011110922.GD1830@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 11 Oct 2011 13:09:22 +0200
> Since commit 2c8cec5c (ipv4: Cache learned PMTU information in inetpeer)
> we cache the learned pmtu informations in inetpeer and propagate these
> informations with the dst_ops->check() functions. However, these functions
> might not be called for udp and raw packets. As a consequence, we don't
> use the learned pmtu informations in these cases. With this patch we
> call dst_check() from ip_setup_cork() to propagate the pmtu informations.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
This dst_check() call will only do something if dst->obsolete is non-zero.
If dst->obsolete can be set in these circumstances, that's a bug. The
caller is responsible for providing either a freshly looked up route
or a cached route which has had dst_check() or sk_dst_check() invoked
upon it.
I am pretty sure these rules are followed by the current code.
Again, there are only two scenerios:
1) 'rt' is just looked up by caller (f.e. udp_sendmsg() in rt == NULL case),
here dst->obsolete is very unlikely to be non-zero.
2) Connected case, and we use cached route from the socket, but here
we'll use sk_dst_check() to validate the route. sk_dst_check()
makes the necessary dst->ops->check() call if dst->obsolete is
non-zero, and in fact that is it's one and only job.
So I cannot see a case where your new check can be necessary.
^ permalink raw reply
* Re: [PATCH 2/4] ipv4: Update pmtu informations on inetpeer only for output routes
From: David Miller @ 2011-10-12 21:08 UTC (permalink / raw)
To: steffen.klassert; +Cc: netdev
In-Reply-To: <20111011111027.GE1830@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 11 Oct 2011 13:10:27 +0200
> @@ -1817,9 +1819,14 @@ static void rt_init_metrics(struct rtable *rt, const struct flowi4 *fl4,
> if (inet_metrics_new(peer))
> memcpy(peer->metrics, fi->fib_metrics,
> sizeof(u32) * RTAX_MAX);
> - dst_init_metrics(&rt->dst, peer->metrics, false);
>
> - check_peer_pmtu(&rt->dst, peer);
> + dst_init_metrics(dst, peer->metrics, false);
> + check_peer_pmtu(dst, peer);
> +
> + if (rt_is_input_route(rt))
> + dst_metric_set(dst, RTAX_MTU,
> + dst->ops->default_mtu(dst));
> +
You really can't do this, it's going to kill all of the memory savings from
storing metrics in the inetpeer cache.
Every input route is going to have it's metrics COW'd with this change.
The whole idea is to use defaults as heavily as possible, and that's
the entire reason why the dst->ops->default_mtu() method exists, so
that we can just leave the values alone and have read-only copies %99
of the time.
Please rearrange your fix so that these goals are still achieved.
Thanks.
^ permalink raw reply
* [PATCH] [TRIVIAL] isdn: hisax: Fix typo 'HISAX_DE_AOC'
From: Paul Bolle @ 2011-10-12 21:19 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-kernel, Karsten Keil, netdev
That should probably be 'CONFIG_DE_AOC'.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
This is just the most obvious way to reconcile an unused Kconfig symbol
(DE_AOC) and an unknown macro (HISAX_DE_AOC). But it's basically just
educated guesswork. Entirely untested too. Added maintainer and netdev,
since this might be stretching the definition of a trivial patch.
drivers/isdn/hisax/l3dss1.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/hisax/l3dss1.c b/drivers/isdn/hisax/l3dss1.c
index b0d9ab1..6a8acf6 100644
--- a/drivers/isdn/hisax/l3dss1.c
+++ b/drivers/isdn/hisax/l3dss1.c
@@ -353,7 +353,7 @@ l3dss1_parse_facility(struct PStack *st, struct l3_process *pc,
{ l3dss1_dummy_invoke(st, cr, id, ident, p, nlen);
return;
}
-#ifdef HISAX_DE_AOC
+#ifdef CONFIG_DE_AOC
{
#define FOO1(s,a,b) \
@@ -422,9 +422,9 @@ l3dss1_parse_facility(struct PStack *st, struct l3_process *pc,
#undef FOO1
}
-#else /* not HISAX_DE_AOC */
+#else /* not CONFIG_DE_AOC */
l3_debug(st, "invoke break");
-#endif /* not HISAX_DE_AOC */
+#endif /* not CONFIG_DE_AOC */
break;
case 2: /* return result */
/* if no process available handle separately */
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH] bonding: L2L3 xmit doesn't support IPv6
From: John Eaglesham @ 2011-10-12 21:15 UTC (permalink / raw)
To: Eric Dumazet
Cc: Yinglin Sun, Andy Gospodarek, Jay Vosburgh,
netdev@vger.kernel.org, linux
In-Reply-To: <1318392395.3686.12.camel@edumazet-laptop>
Eric Dumazet wrote:
> Le mardi 11 octobre 2011 à 20:39 -0700, Yinglin Sun a écrit :
>> On Tue, Oct 11, 2011 at 7:51 PM, Andy Gospodarek <andy@greyhouse.net> wrote:
>>> if (skb->protocol == htons(ETH_P_IP)) {
>>> + struct iphdr *iph = ip_hdr(skb);
>>> + __be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
>>> if (!ip_is_fragment(iph) &&
>>> (iph->protocol == IPPROTO_TCP ||
>>> iph->protocol == IPPROTO_UDP)) {
>>> @@ -3398,7 +3407,18 @@ static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
>>> }
>>> return (layer4_xor ^
>>> ((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
>>> -
>>> + } else if (skb->protocol == htons(ETH_P_IPV6)) {
>>> + struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>>> + __be16 *layer4hdrv6 = (__be16 *)((u8 *)ipv6h + sizeof(*ipv6h));
>>> + if (ipv6h->nexthdr == IPPROTO_TCP || ipv6h->nexthdr == IPPROTO_UDP) {
>> Does this work if this is a fragmentation packet? and if
>> ipv6h->nexthdr is not IPPROTO_TCP/IPPROTO_UDP, it doesn't mean this is
>> not TCP/UDP packet. We need to go through the extension header chain
>> and look at the last one. It's likely there are some other extension
>> headers before L4 header.
>>
>
> Its a best effort.
>
> If first header is not IPPROTO_TCP/UDP, I am not sure its wise to spend
> time in hope to find layer4 info (missing anyway in fragments)
>
> By the way I see no bound checking on SKB head : malicious packet could
> make bond_xmit_hash_policy_l34() access unitialized memory.
>
> We have same 'fastpath' in __skb_get_rxhash()
>
>
Thanks for keeping me in the loop on this.
I caught the bounds checking bug and added checks to code I haven't
submitted in yet (I still need to test that it works before I submit a
patch).
I don't like the idea of sticking a loop in this part of the code,
especially one traversing a list of length controlled by outside users
with no guarantee it is properly formed. I ran some tests gathering
traffic and I never saw any packets with headers between the IPv6 and
TCP or UDP, so at least in the real world right now this code should
behave as expected for the vast majority of traffic. Things might change
in the future, but if this code is clear/clean/simple and solves the
problem today, I'd be happier with that than trying to parse a full IPv6
header.
I'll test and post my revised patch (including bounds checking and
documentation updates) in a day or so.
John
^ permalink raw reply
* Re: [patch] cipso: remove an unneeded NULL check in cipso_v4_doi_add()
From: Paul Moore @ 2011-10-12 21:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, kernel-janitors
In-Reply-To: <20111011215549.GC30887@longonot.mountain>
On Wednesday, October 12, 2011 12:55:49 AM Dan Carpenter wrote:
> On Tue, Oct 11, 2011 at 05:20:11PM -0400, Paul Moore wrote:
> > > - if (doi_def == NULL || doi_def->doi == CIPSO_V4_DOI_UNKNOWN)
> > > + if (doi_def->doi == CIPSO_V4_DOI_UNKNOWN)
> > > goto doi_add_return;
> > > for (iter = 0; iter < CIPSO_V4_TAG_MAXCNT; iter++) {
> > > switch (doi_def->tags[iter]) {
> >
> > I'd prefer to keep the NULL check in there as it does afford a little
> > bit of extra safety and this is management code after all, not
> > per-packet processing code, so the extra check should have no
> > observable performance impact.
>
> The dereferences on the lines before mean we would Oops before
> reaching the check ...
Thanks for pointing that out, I missed that when looking at your patch.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* RE: [net-next 10/12] igb: move DMA Coalescing feature code into separate function.
From: Wyborny, Carolyn @ 2011-10-12 22:21 UTC (permalink / raw)
To: David Miller, Kirsher, Jeffrey T
Cc: Sowles, Jacob R, netdev@vger.kernel.org, gospo@redhat.com,
sassmann@redhat.com
In-Reply-To: <20111012.164834.2208432778408483576.davem@davemloft.net>
>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>On Behalf Of David Miller
>Sent: Wednesday, October 12, 2011 1:49 PM
>To: Kirsher, Jeffrey T
>Cc: Sowles, Jacob R; netdev@vger.kernel.org; gospo@redhat.com;
>sassmann@redhat.com
>Subject: Re: [net-next 10/12] igb: move DMA Coalescing feature code into
>separate function.
>
>From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>Date: Tue, 11 Oct 2011 20:38:26 -0700
>
>> From: Jacob Sowles <jacob.r.sowles@intel.com>
>>
>> DMA Coalescing code needs to be executed during reset, if enabled.
>Moved
>> code to separate function and added a call to it in igb_reset.
>>
>> Signed-off-by: Jacob Sowles <jacob.r.sowles@intel.com>
>> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
>Something is wrong here, this patch just adds the new function but no
>actual callers.
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
Yes, you are correct. We had a problem in our process and this patch got inadvertently changed during it, removing a substantial part of it, including the call to the new function. I'm on it and will be submitting a replacement as soon as possible.
Thanks,
Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation
^ permalink raw reply
* Re: [PATCH net-next 0/5] intel: Logging cleanups
From: Jeff Kirsher @ 2011-10-12 22:56 UTC (permalink / raw)
To: Joe Perches
Cc: netdev@vger.kernel.org, e1000-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org
In-Reply-To: <cover.1318436085.git.joe@perches.com>
[-- Attachment #1: Type: text/plain, Size: 921 bytes --]
On Wed, 2011-10-12 at 09:18 -0700, Joe Perches wrote:
> Just some conversions of printks to pr_<level>
> and some trivial defect corrections.
>
> Joe Perches (5):
> e1000e: Convert printks to pr_<level>
> ixgbevf: Convert printks to pr_<level>
> igbvf: Convert printks to pr_<level>
> igb: Convert printks to pr_<level>
> igb: Convert bare printk to pr_notice
>
> drivers/net/ethernet/intel/e1000e/netdev.c | 232 +++++++++------------
> drivers/net/ethernet/intel/igb/e1000_82575.c | 5 +-
> drivers/net/ethernet/intel/igb/igb_main.c | 155 +++++++--------
> drivers/net/ethernet/intel/igbvf/netdev.c | 14 +-
> drivers/net/ethernet/intel/ixgbevf/ethtool.c | 6 +-
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 26 ++--
> 6 files changed, 201 insertions(+), 237 deletions(-)
>
Thanks Joe. I will add these to my queue of patches.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v4] net-netlink: Add a new attribute to expose TOS values via netlink
From: David Miller @ 2011-10-12 23:10 UTC (permalink / raw)
To: eric.dumazet
Cc: muralira, kuznet, jmorris, yoshfuji, kaber, linux-kernel, netdev
In-Reply-To: <1318446934.2644.0.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 12 Oct 2011 21:15:34 +0200
> Le mercredi 12 octobre 2011 à 12:00 -0700, Muraliraja Muniraju a écrit :
>> From: Murali Raja <muralira@google.com>
>>
>> This patch exposes the tos value for the TCP sockets when the TOS flag
>> is requested in the ext_flags for the inet_diag request. This would mainly be
>> used to expose TOS values for both for TCP and UDP sockets. Currently it is
>> supported for TCP. When netlink support for UDP would be added the support
>> to expose the TOS values would alse be done. For IPV4 tos value is exposed
>> and for IPV6 tclass value is exposed.
>>
>> Signed-off-by: Murali Raja <muralira@google.com>
>> ---
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [patch net-2.6] tg3: negate USE_PHYLIB flag check
From: Matt Carlson @ 2011-10-12 23:55 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev@vger.kernel.org, davem@davemloft.net,
eric.dumazet@gmail.com, Matthew Carlson, Michael Chan
In-Reply-To: <1318410041-2476-1-git-send-email-jpirko@redhat.com>
On Wed, Oct 12, 2011 at 02:00:41AM -0700, Jiri Pirko wrote:
> USE_PHYLIB flag in tg3_remove_one() is being checked incorrectly. This
> results tg3_phy_fini->phy_disconnect is never called and when tg3 module
> is removed.
>
> In my case this resulted in panics in phy_state_machine calling function
> phydev->adjust_link.
>
> So correct this check.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Introduced by commit 63c3a66fe6c827a731dcbdee181158b295626f83, entitled
"tg3: Convert u32 flag,flg2,flg3 uses to bitmap".
Acked-by: Matt Carlson <mcarlson@broadcom.com>
> ---
> drivers/net/tg3.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index 4a1374d..c11a2b8 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -15577,7 +15577,7 @@ static void __devexit tg3_remove_one(struct pci_dev *pdev)
>
> cancel_work_sync(&tp->reset_task);
>
> - if (!tg3_flag(tp, USE_PHYLIB)) {
> + if (tg3_flag(tp, USE_PHYLIB)) {
> tg3_phy_fini(tp);
> tg3_mdio_fini(tp);
> }
> --
> 1.7.6.2
>
>
^ permalink raw reply
* Re: [PATCH 2/3] netdev/phy/of: Handle IEEE802.3 clause 45 Ethernet PHYs in of_mdiobus_register()
From: Grant Likely @ 2011-10-13 0:23 UTC (permalink / raw)
To: David Daney; +Cc: devicetree-discuss, linux-kernel, netdev, davem, afleming
In-Reply-To: <1318442783-29058-3-git-send-email-david.daney@cavium.com>
On Wed, Oct 12, 2011 at 11:06:22AM -0700, David Daney wrote:
> Define two new "compatible" values for Ethernet
> PHYs. "ethernet-phy-ieee802.3-c22" and "ethernet-phy-ieee802.3-c45"
> are used to indicate a PHY uses the corresponding protocol.
>
> If a PHY is "compatible" with "ethernet-phy-ieee802.3-c45", we
> indicate this so that get_phy_device() can properly probe the device.
>
> Signed-off-by: David Daney <david.daney@cavium.com>
I'm okay with this binding. I'd like to get opinions from other
developers before it is committed to though.
Acked-by: Grant Likely <grant.likely@secretlab.ca>
g.
> ---
> Documentation/devicetree/bindings/net/phy.txt | 12 +++++++++++-
> drivers/of/of_mdio.c | 4 ++++
> 2 files changed, 15 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index bb8c742..de9cb32 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -16,8 +16,18 @@ Required properties:
>
> Example:
>
> +Optional Properties:
> +
> +- compatible: Compatible list, may contain "ethernet-phy-ieee802.3-c22" or
> + "ethernet-phy-ieee802.3-c45" for PHYs that implement
> + IEEE802.3 clause 22 or IEEE802.3 clause 45
> + specifications. If neither of these are specified, the
> + default is to assume clause 22. The compatible list may
> + also contain other elements.
> +
> ethernet-phy@0 {
> - linux,phandle = <2452000>
> + compatible = "ethernet-phy-ieee802.3-c22";
> + linux,phandle = <2452000>;
> interrupt-parent = <40000>;
> interrupts = <35 1>;
> reg = <0>;
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 7c28e8c..f837a7f 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -79,6 +79,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
> mdio->irq[addr] = PHY_POLL;
> }
>
> + if (of_device_is_compatible(child,
> + "ethernet-phy-ieee802.3-c45"))
> + addr |= MII_ADDR_C45;
> +
> phy = get_phy_device(mdio, addr);
> if (!phy || IS_ERR(phy)) {
> dev_err(&mdio->dev, "error probing PHY at address %i\n",
> --
> 1.7.2.3
>
^ permalink raw reply
* Re: [PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY
From: Grant Likely @ 2011-10-13 0:31 UTC (permalink / raw)
To: David Daney; +Cc: devicetree-discuss, linux-kernel, netdev, davem, afleming
In-Reply-To: <1318442783-29058-4-git-send-email-david.daney@cavium.com>
On Wed, Oct 12, 2011 at 11:06:23AM -0700, David Daney wrote:
> Add a driver and PHY_ID number for said device. This is a 10Gig PHY
> which uses MII_ADDR_C45 addressing, it is always 10G full duplex, so
> there is no autonegotiation. All we do is report link state and send
> interrupts when it changes.
>
> If the PHY has a device tree of_node associated with it, the
> "broadcom,c45-reg-init" property is used to supply register
> initialization values when config_init() is called.
>
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
> .../devicetree/bindings/net/broadcom-bcm8706.txt | 28 +++
> drivers/net/phy/Kconfig | 5 +
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/bcm8706.c | 212 ++++++++++++++++++++
> include/linux/brcmphy.h | 1 +
> 5 files changed, 247 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
> create mode 100644 drivers/net/phy/bcm8706.c
>
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
> new file mode 100644
> index 0000000..d58bea9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
> @@ -0,0 +1,28 @@
> +The Broadcom BCM8706 is a 10G Ethernet PHY. It has these bindings in
> +addition to the standard PHY bindings.
> +
> +Compatible: Should contain "broadcom,bcm8706" and
> + "ethernet-phy-ieee802.3-c45"
> +
> +Optional Properties:
> +
> +- broadcom,c45-reg-init : one of more sets of 4 cells. The first cell
> + is the device type, the second a register address, the third cell
> + contains a mask to be ANDed with the existing register value, and
> + the fourth cell is ORed with he result to yield the new register
> + value.
... a mask value of '0' should also guarantee that the driver does not do a read before the write.
What have we got so far in this regard for other phys and devices? I
don't think it necessary to put 'c45' in the property name. reg-init
should be sufficient. I'd like to hear from others if it would be
valuable to have a 'reg-init-sequence' property of the above format.
What does the device type cell indicate? Wouldn't the driver
naturally have the device id from the address of the cell?
> +static int __init bcm8706_init(void)
> +{
> + int ret;
> +
> + ret = phy_driver_register(&bcm8706_driver);
> +
> + return ret;
> +}
> +module_init(bcm8706_init);
or simply:
static int __init bcm8706_init(void)
{
return phy_driver_register(&bcm8706_driver);
}
module_init(bcm8706_init);
Otherwise the driver code seems to be fine.
g.
^ 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