* Re: [PATCH v2] netlink: align attributes on 64-bits
From: Nicolas Dichtel @ 2012-12-18 10:18 UTC (permalink / raw)
To: David Laight; +Cc: bhutchings, tgraf, netdev, davem
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B70F2@saturn3.aculab.com>
Le 18/12/2012 10:19, David Laight a écrit :
>> Le 17/12/2012 18:06, David Laight a écrit :
>>>> int nla_put(struct sk_buff *skb, int attrtype, int attrlen, const void *data)
>>>> {
>>>> - if (unlikely(skb_tailroom(skb) < nla_total_size(attrlen)))
>>>> + int align = IS_ALIGNED((unsigned long)skb_tail_pointer(skb), 8) ? 0 : 4;
>>>
>>> I've just realised where you are adding this!
>>> You only want to add pad if the attribute is a single 64bit item,
>>> not whenever the destination is misaligned.
>> As said in the commit log, I want to align all attributes. An attribute can be
>> like this:
>>
>> struct foo {
>> __u32 bar1;
>> __u32 bar2;
>> __u64 bar3;
>> }
>>
>> nla_put() don't know what is contained in the attribute.
>
> Put there is no need to 8-byte align something whose size isn't a
> multiple of 8 bytes.
Even if you cast the structure in a buffer and read bar3 (without any memcpy
before)?
>
>>> ...
>>>> + if (align) {
>>>> + /* Goal is to add an attribute with size 4. We know that
>>>> + * NLA_HDRLEN is 4, hence payload is 0.
>>>> + */
>>>> + __nla_reserve(skb, 0, 0);
>>>
>>> One of those zeros should be 'align - 4', then the comment
>>> can be more descriptive.
>
>> I thought if you were to research why we use 0, you would know that the first 0
>> is the type and the second is the payload size...
>
> I can tell that one is the type and the other the size, you've
> implied that the 'type+size' actually total 4 bytes.
> I don't need to find out which is which!
> Now you've told me I'd have written:
> _nla_reserve(skb, 0, align - NLA_HDRLEN);
>
> The compiler could well have tracked the value - so know it is 4.
> OTOH you might want to generate the size of 'align' without
> using a conditional.
Yes, it is the goal ;-)
^ permalink raw reply
* [PATCHv3 2/2] smsc95xx: enable dynamic autosuspend
From: Steve Glendinning @ 2012-12-18 10:46 UTC (permalink / raw)
To: netdev; +Cc: ming.lei, oneukum, gregkh, Steve Glendinning
In-Reply-To: <1355827574-15164-1-git-send-email-steve.glendinning@shawell.net>
This patch enables USB dynamic autosuspend for LAN9500A. This
saves very little power in itself, but it allows power saving
in upstream hubs/hosts.
The earlier devices in this family (LAN9500/9512/9514) do not
support this feature.
Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
drivers/net/usb/smsc95xx.c | 151 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 150 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 124e67f..6a74a68 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -55,6 +55,13 @@
#define FEATURE_PHY_NLP_CROSSOVER (0x02)
#define FEATURE_AUTOSUSPEND (0x04)
+#define SUSPEND_SUSPEND0 (0x01)
+#define SUSPEND_SUSPEND1 (0x02)
+#define SUSPEND_SUSPEND2 (0x04)
+#define SUSPEND_SUSPEND3 (0x08)
+#define SUSPEND_ALLMODES (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \
+ SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3)
+
struct smsc95xx_priv {
u32 mac_cr;
u32 hash_hi;
@@ -62,6 +69,7 @@ struct smsc95xx_priv {
u32 wolopts;
spinlock_t mac_cr_lock;
u8 features;
+ u8 suspend_flags;
};
static bool turbo_mode = true;
@@ -1242,6 +1250,8 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
/* read back PM_CTRL */
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
+ pdata->suspend_flags |= SUSPEND_SUSPEND0;
+
return ret;
}
@@ -1286,11 +1296,14 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+ pdata->suspend_flags |= SUSPEND_SUSPEND1;
+
return ret;
}
static int smsc95xx_enter_suspend2(struct usbnet *dev)
{
+ struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
u32 val;
int ret;
@@ -1303,9 +1316,97 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev)
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+ pdata->suspend_flags |= SUSPEND_SUSPEND2;
+
return ret;
}
+static int smsc95xx_enter_suspend3(struct usbnet *dev)
+{
+ struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ u32 val;
+ int ret;
+
+ ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val);
+ if (ret < 0)
+ return ret;
+
+ if (val & 0xFFFF) {
+ netdev_info(dev->net, "rx fifo not empty in autosuspend\n");
+ return -EBUSY;
+ }
+
+ ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
+ if (ret < 0)
+ return ret;
+
+ val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
+ val |= PM_CTL_SUS_MODE_3 | PM_CTL_RES_CLR_WKP_STS;
+
+ ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+ if (ret < 0)
+ return ret;
+
+ /* clear wol status */
+ val &= ~PM_CTL_WUPS_;
+ val |= PM_CTL_WUPS_WOL_;
+
+ ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
+ if (ret < 0)
+ return ret;
+
+ pdata->suspend_flags |= SUSPEND_SUSPEND3;
+
+ return 0;
+}
+
+static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up)
+{
+ struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ int ret;
+
+ if (!netif_running(dev->net)) {
+ /* interface is ifconfig down so fully power down hw */
+ netdev_dbg(dev->net, "autosuspend entering SUSPEND2\n");
+ return smsc95xx_enter_suspend2(dev);
+ }
+
+ if (!link_up) {
+ /* link is down so enter EDPD mode, but only if device can
+ * reliably resume from it. This check should be redundant
+ * as current FEATURE_AUTOSUSPEND parts also support
+ * FEATURE_PHY_NLP_CROSSOVER but it's included for clarity */
+ if (!(pdata->features & FEATURE_PHY_NLP_CROSSOVER)) {
+ netdev_warn(dev->net, "EDPD not supported\n");
+ return -EBUSY;
+ }
+
+ netdev_dbg(dev->net, "autosuspend entering SUSPEND1\n");
+
+ /* enable PHY wakeup events for if cable is attached */
+ ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
+ PHY_INT_MASK_ANEG_COMP_);
+ if (ret < 0) {
+ netdev_warn(dev->net, "error enabling PHY wakeup ints\n");
+ return ret;
+ }
+
+ netdev_info(dev->net, "entering SUSPEND1 mode\n");
+ return smsc95xx_enter_suspend1(dev);
+ }
+
+ /* enable PHY wakeup events so we remote wakeup if cable is pulled */
+ ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
+ PHY_INT_MASK_LINK_DOWN_);
+ if (ret < 0) {
+ netdev_warn(dev->net, "error enabling PHY wakeup ints\n");
+ return ret;
+ }
+
+ netdev_dbg(dev->net, "autosuspend entering SUSPEND3\n");
+ return smsc95xx_enter_suspend3(dev);
+}
+
static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
{
struct usbnet *dev = usb_get_intfdata(intf);
@@ -1313,15 +1414,35 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
u32 val, link_up;
int ret;
+ /* TODO: don't indicate this feature to usb framework if
+ * our current hardware doesn't have the capability
+ */
+ if ((message.event == PM_EVENT_AUTO_SUSPEND) &&
+ (!(pdata->features & FEATURE_AUTOSUSPEND))) {
+ netdev_warn(dev->net, "autosuspend not supported\n");
+ return -EBUSY;
+ }
+
ret = usbnet_suspend(intf, message);
if (ret < 0) {
netdev_warn(dev->net, "usbnet_suspend error\n");
return ret;
}
+ if (pdata->suspend_flags) {
+ netdev_warn(dev->net, "error during last resume\n");
+ pdata->suspend_flags = 0;
+ }
+
/* determine if link is up using only _nopm functions */
link_up = smsc95xx_link_ok_nopm(dev);
+ if (message.event == PM_EVENT_AUTO_SUSPEND) {
+ ret = smsc95xx_autosuspend(dev, link_up);
+ goto done;
+ }
+
+ /* if we get this far we're not autosuspending */
/* if no wol options set, or if link is down and we're not waking on
* PHY activity, enter lowest power SUSPEND2 mode
*/
@@ -1552,12 +1673,18 @@ static int smsc95xx_resume(struct usb_interface *intf)
{
struct usbnet *dev = usb_get_intfdata(intf);
struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+ u8 suspend_flags = pdata->suspend_flags;
int ret;
u32 val;
BUG_ON(!dev);
- if (pdata->wolopts) {
+ netdev_dbg(dev->net, "resume suspend_flags=0x%02x\n", suspend_flags);
+
+ /* do this first to ensure it's cleared even in error case */
+ pdata->suspend_flags = 0;
+
+ if (suspend_flags & SUSPEND_ALLMODES) {
/* clear wake-up sources */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
if (ret < 0)
@@ -1741,6 +1868,26 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
return skb;
}
+static int smsc95xx_manage_power(struct usbnet *dev, int on)
+{
+ struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+
+ dev->intf->needs_remote_wakeup = on;
+
+ if (pdata->features & FEATURE_AUTOSUSPEND)
+ return 0;
+
+ /* this chip revision doesn't support autosuspend */
+ netdev_info(dev->net, "hardware doesn't support USB autosuspend\n");
+
+ if (on)
+ usb_autopm_get_interface_no_resume(dev->intf);
+ else
+ usb_autopm_put_interface(dev->intf);
+
+ return 0;
+}
+
static const struct driver_info smsc95xx_info = {
.description = "smsc95xx USB 2.0 Ethernet",
.bind = smsc95xx_bind,
@@ -1750,6 +1897,7 @@ static const struct driver_info smsc95xx_info = {
.rx_fixup = smsc95xx_rx_fixup,
.tx_fixup = smsc95xx_tx_fixup,
.status = smsc95xx_status,
+ .manage_power = smsc95xx_manage_power,
.flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_LINK_INTR,
};
@@ -1857,6 +2005,7 @@ static struct usb_driver smsc95xx_driver = {
.reset_resume = smsc95xx_resume,
.disconnect = usbnet_disconnect,
.disable_hub_initiated_lpm = 1,
+ .supports_autosuspend = 1,
};
module_usb_driver(smsc95xx_driver);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/2] smsc95xx: eliminate duplicate warnings on io failure
From: Steve Glendinning @ 2012-12-18 10:46 UTC (permalink / raw)
To: netdev; +Cc: ming.lei, oneukum, gregkh, Steve Glendinning, Joe Perches
In-Reply-To: <1355827574-15164-1-git-send-email-steve.glendinning@shawell.net>
The register read/write functions already log a warning if
an access fails, so this patch removes the additional warnings
logged by callers that don't add any more information.
This patch makes the resulting driver smaller by not containing
as many warning strings.
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
drivers/net/usb/smsc95xx.c | 284 +++++++++++---------------------------------
1 file changed, 67 insertions(+), 217 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 9b73670..124e67f 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -513,10 +513,8 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
u32 flow, afc_cfg = 0;
int ret = smsc95xx_read_reg(dev, AFC_CFG, &afc_cfg);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading AFC_CFG\n");
+ if (ret < 0)
return ret;
- }
if (duplex == DUPLEX_FULL) {
u8 cap = mii_resolve_flowctrl_fdx(lcladv, rmtadv);
@@ -541,16 +539,10 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
}
ret = smsc95xx_write_reg(dev, FLOW, flow);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing FLOW\n");
- return ret;
- }
-
- ret = smsc95xx_write_reg(dev, AFC_CFG, afc_cfg);
if (ret < 0)
- netdev_warn(dev->net, "Error writing AFC_CFG\n");
+ return ret;
- return ret;
+ return smsc95xx_write_reg(dev, AFC_CFG, afc_cfg);
}
static int smsc95xx_link_reset(struct usbnet *dev)
@@ -564,16 +556,12 @@ static int smsc95xx_link_reset(struct usbnet *dev)
/* clear interrupt status */
ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading PHY_INT_SRC\n");
+ if (ret < 0)
return ret;
- }
ret = smsc95xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL_);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing INT_STS\n");
+ if (ret < 0)
return ret;
- }
mii_check_media(mii, 1, 1);
mii_ethtool_gset(&dev->mii, &ecmd);
@@ -595,10 +583,8 @@ static int smsc95xx_link_reset(struct usbnet *dev)
spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
ret = smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing MAC_CR\n");
+ if (ret < 0)
return ret;
- }
ret = smsc95xx_phy_update_flowcontrol(dev, ecmd.duplex, lcladv, rmtadv);
if (ret < 0)
@@ -638,10 +624,8 @@ static int smsc95xx_set_features(struct net_device *netdev,
int ret;
ret = smsc95xx_read_reg(dev, COE_CR, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read COE_CR: %d\n", ret);
+ if (ret < 0)
return ret;
- }
if (features & NETIF_F_HW_CSUM)
read_buf |= Tx_COE_EN_;
@@ -654,10 +638,8 @@ static int smsc95xx_set_features(struct net_device *netdev,
read_buf &= ~Rx_COE_EN_;
ret = smsc95xx_write_reg(dev, COE_CR, read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write COE_CR: %d\n", ret);
+ if (ret < 0)
return ret;
- }
netif_dbg(dev, hw, dev->net, "COE_CR = 0x%08x\n", read_buf);
return 0;
@@ -800,16 +782,10 @@ static int smsc95xx_set_mac_address(struct usbnet *dev)
int ret;
ret = smsc95xx_write_reg(dev, ADDRL, addr_lo);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write ADDRL: %d\n", ret);
- return ret;
- }
-
- ret = smsc95xx_write_reg(dev, ADDRH, addr_hi);
if (ret < 0)
- netdev_warn(dev->net, "Failed to write ADDRH: %d\n", ret);
+ return ret;
- return ret;
+ return smsc95xx_write_reg(dev, ADDRH, addr_hi);
}
/* starts the TX path */
@@ -825,17 +801,11 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
ret = smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write MAC_CR: %d\n", ret);
+ if (ret < 0)
return ret;
- }
/* Enable Tx at SCSRs */
- ret = smsc95xx_write_reg(dev, TX_CFG, TX_CFG_ON_);
- if (ret < 0)
- netdev_warn(dev->net, "Failed to write TX_CFG: %d\n", ret);
-
- return ret;
+ return smsc95xx_write_reg(dev, TX_CFG, TX_CFG_ON_);
}
/* Starts the Receive path */
@@ -843,17 +813,12 @@ static int smsc95xx_start_rx_path(struct usbnet *dev, int in_pm)
{
struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
unsigned long flags;
- int ret;
spin_lock_irqsave(&pdata->mac_cr_lock, flags);
pdata->mac_cr |= MAC_CR_RXEN_;
spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
- ret = __smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr, in_pm);
- if (ret < 0)
- netdev_warn(dev->net, "Failed to write MAC_CR: %d\n", ret);
-
- return ret;
+ return __smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr, in_pm);
}
static int smsc95xx_phy_initialize(struct usbnet *dev)
@@ -910,19 +875,15 @@ static int smsc95xx_reset(struct usbnet *dev)
netif_dbg(dev, ifup, dev->net, "entering smsc95xx_reset\n");
ret = smsc95xx_write_reg(dev, HW_CFG, HW_CFG_LRST_);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write HW_CFG_LRST_ bit in HW_CFG\n");
+ if (ret < 0)
return ret;
- }
timeout = 0;
do {
msleep(10);
ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+ if (ret < 0)
return ret;
- }
timeout++;
} while ((read_buf & HW_CFG_LRST_) && (timeout < 100));
@@ -932,19 +893,15 @@ static int smsc95xx_reset(struct usbnet *dev)
}
ret = smsc95xx_write_reg(dev, PM_CTRL, PM_CTL_PHY_RST_);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write PM_CTRL: %d\n", ret);
+ if (ret < 0)
return ret;
- }
timeout = 0;
do {
msleep(10);
ret = smsc95xx_read_reg(dev, PM_CTRL, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read PM_CTRL: %d\n", ret);
+ if (ret < 0)
return ret;
- }
timeout++;
} while ((read_buf & PM_CTL_PHY_RST_) && (timeout < 100));
@@ -961,10 +918,8 @@ static int smsc95xx_reset(struct usbnet *dev)
dev->net->dev_addr);
ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+ if (ret < 0)
return ret;
- }
netif_dbg(dev, ifup, dev->net, "Read Value from HW_CFG : 0x%08x\n",
read_buf);
@@ -972,16 +927,12 @@ static int smsc95xx_reset(struct usbnet *dev)
read_buf |= HW_CFG_BIR_;
ret = smsc95xx_write_reg(dev, HW_CFG, read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write HW_CFG_BIR_ bit in HW_CFG\n");
+ if (ret < 0)
return ret;
- }
ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+ if (ret < 0)
return ret;
- }
netif_dbg(dev, ifup, dev->net,
"Read Value from HW_CFG after writing HW_CFG_BIR_: 0x%08x\n",
@@ -1002,42 +953,32 @@ static int smsc95xx_reset(struct usbnet *dev)
(ulong)dev->rx_urb_size);
ret = smsc95xx_write_reg(dev, BURST_CAP, burst_cap);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write BURST_CAP: %d\n", ret);
+ if (ret < 0)
return ret;
- }
ret = smsc95xx_read_reg(dev, BURST_CAP, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read BURST_CAP: %d\n", ret);
+ if (ret < 0)
return ret;
- }
netif_dbg(dev, ifup, dev->net,
"Read Value from BURST_CAP after writing: 0x%08x\n",
read_buf);
ret = smsc95xx_write_reg(dev, BULK_IN_DLY, DEFAULT_BULK_IN_DELAY);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write BULK_IN_DLY: %d\n", ret);
+ if (ret < 0)
return ret;
- }
ret = smsc95xx_read_reg(dev, BULK_IN_DLY, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read BULK_IN_DLY: %d\n", ret);
+ if (ret < 0)
return ret;
- }
netif_dbg(dev, ifup, dev->net,
"Read Value from BULK_IN_DLY after writing: 0x%08x\n",
read_buf);
ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+ if (ret < 0)
return ret;
- }
netif_dbg(dev, ifup, dev->net, "Read Value from HW_CFG: 0x%08x\n",
read_buf);
@@ -1051,69 +992,51 @@ static int smsc95xx_reset(struct usbnet *dev)
read_buf |= NET_IP_ALIGN << 9;
ret = smsc95xx_write_reg(dev, HW_CFG, read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write HW_CFG: %d\n", ret);
+ if (ret < 0)
return ret;
- }
ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+ if (ret < 0)
return ret;
- }
netif_dbg(dev, ifup, dev->net,
"Read Value from HW_CFG after writing: 0x%08x\n", read_buf);
ret = smsc95xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL_);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write INT_STS: %d\n", ret);
+ if (ret < 0)
return ret;
- }
ret = smsc95xx_read_reg(dev, ID_REV, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read ID_REV: %d\n", ret);
+ if (ret < 0)
return ret;
- }
netif_dbg(dev, ifup, dev->net, "ID_REV = 0x%08x\n", read_buf);
/* Configure GPIO pins as LED outputs */
write_buf = LED_GPIO_CFG_SPD_LED | LED_GPIO_CFG_LNK_LED |
LED_GPIO_CFG_FDX_LED;
ret = smsc95xx_write_reg(dev, LED_GPIO_CFG, write_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write LED_GPIO_CFG: %d\n", ret);
+ if (ret < 0)
return ret;
- }
/* Init Tx */
ret = smsc95xx_write_reg(dev, FLOW, 0);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write FLOW: %d\n", ret);
+ if (ret < 0)
return ret;
- }
ret = smsc95xx_write_reg(dev, AFC_CFG, AFC_CFG_DEFAULT);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write AFC_CFG: %d\n", ret);
+ if (ret < 0)
return ret;
- }
/* Don't need mac_cr_lock during initialisation */
ret = smsc95xx_read_reg(dev, MAC_CR, &pdata->mac_cr);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read MAC_CR: %d\n", ret);
+ if (ret < 0)
return ret;
- }
/* Init Rx */
/* Set Vlan */
ret = smsc95xx_write_reg(dev, VLAN1, (u32)ETH_P_8021Q);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write VLAN1: %d\n", ret);
+ if (ret < 0)
return ret;
- }
/* Enable or disable checksum offload engines */
ret = smsc95xx_set_features(dev->net, dev->net->features);
@@ -1131,19 +1054,15 @@ static int smsc95xx_reset(struct usbnet *dev)
}
ret = smsc95xx_read_reg(dev, INT_EP_CTL, &read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read INT_EP_CTL: %d\n", ret);
+ if (ret < 0)
return ret;
- }
/* enable PHY interrupts */
read_buf |= INT_EP_CTL_PHY_INT_;
ret = smsc95xx_write_reg(dev, INT_EP_CTL, read_buf);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to write INT_EP_CTL: %d\n", ret);
+ if (ret < 0)
return ret;
- }
ret = smsc95xx_start_tx_path(dev);
if (ret < 0) {
@@ -1213,10 +1132,8 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
/* detect device revision as different features may be available */
ret = smsc95xx_read_reg(dev, ID_REV, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Failed to read ID_REV: %d\n", ret);
+ if (ret < 0)
return ret;
- }
val >>= 16;
if ((val == ID_REV_CHIP_ID_9500A_) || (val == ID_REV_CHIP_ID_9530_) ||
@@ -1261,17 +1178,13 @@ static int smsc95xx_enable_phy_wakeup_interrupts(struct usbnet *dev, u16 mask)
/* read to clear */
ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_INT_SRC);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading PHY_INT_SRC\n");
+ if (ret < 0)
return ret;
- }
/* enable interrupt source */
ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_INT_MASK);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading PHY_INT_MASK\n");
+ if (ret < 0)
return ret;
- }
ret |= mask;
@@ -1287,16 +1200,12 @@ static int smsc95xx_link_ok_nopm(struct usbnet *dev)
/* first, a dummy read, needed to latch some MII phys */
ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, MII_BMSR);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading MII_BMSR\n");
+ if (ret < 0)
return ret;
- }
ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, MII_BMSR);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading MII_BMSR\n");
+ if (ret < 0)
return ret;
- }
return !!(ret & BMSR_LSTATUS);
}
@@ -1308,19 +1217,15 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
int ret;
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading PM_CTRL\n");
+ if (ret < 0)
return ret;
- }
val &= (~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_));
val |= PM_CTL_SUS_MODE_0;
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing PM_CTRL\n");
+ if (ret < 0)
return ret;
- }
/* clear wol status */
val &= ~PM_CTL_WUPS_;
@@ -1331,15 +1236,11 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
val |= PM_CTL_WUPS_ED_;
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing PM_CTRL\n");
+ if (ret < 0)
return ret;
- }
/* read back PM_CTRL */
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
- if (ret < 0)
- netdev_warn(dev->net, "Error reading PM_CTRL\n");
return ret;
}
@@ -1360,10 +1261,8 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
/* enable energy detect power-down mode */
ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_MODE_CTRL_STS);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading PHY_MODE_CTRL_STS\n");
+ if (ret < 0)
return ret;
- }
ret |= MODE_CTRL_STS_EDPWRDOWN_;
@@ -1371,27 +1270,21 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
/* enter SUSPEND1 mode */
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading PM_CTRL\n");
+ if (ret < 0)
return ret;
- }
val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
val |= PM_CTL_SUS_MODE_1;
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing PM_CTRL\n");
+ if (ret < 0)
return ret;
- }
/* clear wol status, enable energy detection */
val &= ~PM_CTL_WUPS_;
val |= (PM_CTL_WUPS_ED_ | PM_CTL_ED_EN_);
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
- if (ret < 0)
- netdev_warn(dev->net, "Error writing PM_CTRL\n");
return ret;
}
@@ -1402,17 +1295,13 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev)
int ret;
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading PM_CTRL\n");
+ if (ret < 0)
return ret;
- }
val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
val |= PM_CTL_SUS_MODE_2;
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
- if (ret < 0)
- netdev_warn(dev->net, "Error writing PM_CTRL\n");
return ret;
}
@@ -1442,32 +1331,24 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
/* disable energy detect (link up) & wake up events */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading WUCSR\n");
+ if (ret < 0)
goto done;
- }
val &= ~(WUCSR_MPEN_ | WUCSR_WAKE_EN_);
ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing WUCSR\n");
+ if (ret < 0)
goto done;
- }
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading PM_CTRL\n");
+ if (ret < 0)
goto done;
- }
val &= ~(PM_CTL_ED_EN_ | PM_CTL_WOL_EN_);
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing PM_CTRL\n");
+ if (ret < 0)
goto done;
- }
ret = smsc95xx_enter_suspend2(dev);
goto done;
@@ -1565,7 +1446,6 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
for (i = 0; i < (wuff_filter_count * 4); i++) {
ret = smsc95xx_write_reg_nopm(dev, WUFF, filter_mask[i]);
if (ret < 0) {
- netdev_warn(dev->net, "Error writing WUFF\n");
kfree(filter_mask);
goto done;
}
@@ -1574,67 +1454,51 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
for (i = 0; i < (wuff_filter_count / 4); i++) {
ret = smsc95xx_write_reg_nopm(dev, WUFF, command[i]);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing WUFF\n");
+ if (ret < 0)
goto done;
- }
}
for (i = 0; i < (wuff_filter_count / 4); i++) {
ret = smsc95xx_write_reg_nopm(dev, WUFF, offset[i]);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing WUFF\n");
+ if (ret < 0)
goto done;
- }
}
for (i = 0; i < (wuff_filter_count / 2); i++) {
ret = smsc95xx_write_reg_nopm(dev, WUFF, crc[i]);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing WUFF\n");
+ if (ret < 0)
goto done;
- }
}
/* clear any pending pattern match packet status */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading WUCSR\n");
+ if (ret < 0)
goto done;
- }
val |= WUCSR_WUFR_;
ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing WUCSR\n");
+ if (ret < 0)
goto done;
- }
}
if (pdata->wolopts & WAKE_MAGIC) {
/* clear any pending magic packet status */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading WUCSR\n");
+ if (ret < 0)
goto done;
- }
val |= WUCSR_MPR_;
ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing WUCSR\n");
+ if (ret < 0)
goto done;
- }
}
/* enable/disable wakeup sources */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading WUCSR\n");
+ if (ret < 0)
goto done;
- }
if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
netdev_info(dev->net, "enabling pattern match wakeup\n");
@@ -1653,17 +1517,13 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
}
ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing WUCSR\n");
+ if (ret < 0)
goto done;
- }
/* enable wol wakeup source */
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading PM_CTRL\n");
+ if (ret < 0)
goto done;
- }
val |= PM_CTL_WOL_EN_;
@@ -1672,10 +1532,8 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
val |= PM_CTL_ED_EN_;
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing PM_CTRL\n");
+ if (ret < 0)
goto done;
- }
/* enable receiver to enable frame reception */
smsc95xx_start_rx_path(dev, 1);
@@ -1702,34 +1560,26 @@ static int smsc95xx_resume(struct usb_interface *intf)
if (pdata->wolopts) {
/* clear wake-up sources */
ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading WUCSR\n");
+ if (ret < 0)
return ret;
- }
val &= ~(WUCSR_WAKE_EN_ | WUCSR_MPEN_);
ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing WUCSR\n");
+ if (ret < 0)
return ret;
- }
/* clear wake-up status */
ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error reading PM_CTRL\n");
+ if (ret < 0)
return ret;
- }
val &= ~PM_CTL_WOL_EN_;
val |= PM_CTL_WUPS_;
ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
- if (ret < 0) {
- netdev_warn(dev->net, "Error writing PM_CTRL\n");
+ if (ret < 0)
return ret;
- }
}
ret = usbnet_resume(intf);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/2] smsc95xx enhancements
From: Steve Glendinning @ 2012-12-18 10:46 UTC (permalink / raw)
To: netdev; +Cc: ming.lei, oneukum, gregkh, Steve Glendinning
Two driver enhancements for net-next. There's ongoing discussion around
the best way to improve autosuspend moving forwards but in the meantime
the second patch in this set enables autosuspend for supported parts in
most situations.
Steve Glendinning (2):
smsc95xx: eliminate duplicate warnings on io failure
smsc95xx: enable dynamic autosuspend
drivers/net/usb/smsc95xx.c | 435 ++++++++++++++++++++++----------------------
1 file changed, 217 insertions(+), 218 deletions(-)
--
1.7.10.4
^ permalink raw reply
* [PATCH] ethtool: add register dump support for SMSC LAN9420
From: Steve Glendinning @ 2012-12-18 9:34 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Steve Glendinning
This patch adds support for SMSC's LAN9420 PCI ethernet controller
to ethtool's dump registers (-d) command.
This patch is for use with the smsc9420 driver.
Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
Makefile.am | 2 +-
ethtool.c | 2 ++
internal.h | 3 ++
smsc9420.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 101 insertions(+), 1 deletion(-)
create mode 100644 smsc9420.c
diff --git a/Makefile.am b/Makefile.am
index ba1faa6..728be0a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -10,7 +10,7 @@ ethtool_SOURCES = ethtool.c ethtool-copy.h internal.h net_tstamp-copy.h \
fec_8xx.c ibm_emac.c ixgb.c ixgbe.c natsemi.c \
pcnet32.c realtek.c tg3.c marvell.c vioc.c \
smsc911x.c at76c50x-usb.c sfc.c stmmac.c \
- rxclass.c sfpid.c sfpdiag.c
+ rxclass.c sfpid.c sfpdiag.c smsc9420.c
TESTS = test-cmdline test-features
check_PROGRAMS = test-cmdline test-features
diff --git a/ethtool.c b/ethtool.c
index 345c21c..97c3d76 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -15,6 +15,7 @@
* amd8111e support by Reeja John <reeja.john@amd.com>
* long arguments by Andi Kleen.
* SMSC LAN911x support by Steve Glendinning <steve.glendinning@smsc.com>
+ * SMSC LAN9420 support by Steve Glendinning <steve.glendinning@shawell.net>
* Rx Network Flow Control configuration support <santwona.behera@sun.com>
* Various features by Ben Hutchings <bhutchings@solarflare.com>;
* Copyright 2009, 2010 Solarflare Communications
@@ -884,6 +885,7 @@ static const struct {
{ "sky2", sky2_dump_regs },
{ "vioc", vioc_dump_regs },
{ "smsc911x", smsc911x_dump_regs },
+ { "smsc9420", smsc9420_dump_regs },
{ "at76c50x-usb", at76c50x_usb_dump_regs },
{ "sfc", sfc_dump_regs },
{ "st_mac100", st_mac100_dump_regs },
diff --git a/internal.h b/internal.h
index e977a81..8873cd1 100644
--- a/internal.h
+++ b/internal.h
@@ -228,6 +228,9 @@ int vioc_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
/* SMSC LAN911x/LAN921x embedded ethernet controller */
int smsc911x_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
+/* SMSC LAN9420 PCI ethernet controller */
+int smsc9420_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
+
int at76c50x_usb_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
/* Solarflare Solarstorm controllers */
diff --git a/smsc9420.c b/smsc9420.c
new file mode 100644
index 0000000..b6a24a0
--- /dev/null
+++ b/smsc9420.c
@@ -0,0 +1,95 @@
+#include <stdio.h>
+#include <string.h>
+#include "internal.h"
+
+int smsc9420_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
+{
+ unsigned int *smsc_reg = (unsigned int *)regs->data;
+
+ fprintf(stdout, "LAN9420 DMAC Control & Status Registers\n");
+ fprintf(stdout, "offset 0x00, BUS_MODE = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x04, TX_POLL_DEMAND = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x08, TX_POLL_DEMAND = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x0C, RX_BASE_ADDR = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x10, TX_BASE_ADDR = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x14, DMAC_STATUS = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x18, DMAC_CONTROL = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x1C, DMAC_INTR_ENA = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x20, MISS_FRAME_CNT = 0x%08X\n",*smsc_reg++);
+ smsc_reg += 11; /* 0x24 - 0x4C RESERVED */
+ fprintf(stdout, "offset 0x50, CUR_TX_BUF_ADDR = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x54, CUR_RX_BUF_ADDR = 0x%08X\n",*smsc_reg++);
+ smsc_reg += 10; /* 0x58 - 0x7C RESERVED */
+ fprintf(stdout, "\n");
+
+ fprintf(stdout, "LAN9420 MAC Control & Status Registers\n");
+ fprintf(stdout, "offset 0x80, MAC_CR = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x84, ADDRH = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x88, ADDRL = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x8C, HASHH = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x90, HASHL = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x94, MIIADDR = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x98, MIIDATA = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0x9C, FLOW = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xA0, VLAN1 = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xA4, VLAN2 = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xA8, WUFF = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xAC, WUCSR = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xB0, COE_CR = 0x%08X\n",*smsc_reg++);
+ smsc_reg += 3; /* 0xB4 - 0xBC RESERVED */
+ fprintf(stdout, "\n");
+
+ fprintf(stdout, "LAN9420 System Control & Status Registers\n");
+ fprintf(stdout, "offset 0xC0, ID_REV = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xC4, INT_CTL = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xC8, INT_STS = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xCC, INT_CFG = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xD0, GPIO_CFG = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xD4, GPT_CFG = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xD8, GPT_CNT = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xDC, BUS_CFG = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xE0, PMT_CTRL = 0x%08X\n",*smsc_reg++);
+ smsc_reg += 4; /* 0xE4 - 0xF0 RESERVED */
+ fprintf(stdout, "offset 0xF4, FREE_RUN = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xF8, E2P_CMD = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "offset 0xFC, E2P_DATA = 0x%08X\n",*smsc_reg++);
+ fprintf(stdout, "\n");
+
+ fprintf(stdout, "PHY Registers\n");
+ fprintf(stdout, "index 0, Basic Control Reg = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 1, Basic Status Reg = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 2, PHY identifier 1 = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 3, PHY identifier 2 = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 4, Auto Negotiation Advertisement Reg = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 5, Auto Negotiation Link Partner Ability Reg = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 6, Auto Negotiation Expansion Register = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 7, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 8, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 9, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 10, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 11, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 12, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 13, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 14, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 15, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 16, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 17, Mode Control/Status Reg = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 18, Special Modes = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 19, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 20, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 21, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 22, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 23, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 24, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 25, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 26, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 27, Control/Status Indication = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 28, Reserved = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 29, Interrupt Source Register = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 30, Interrupt Mask Register = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "index 31, PHY Special Control/Status Register = 0x%04X\n",*smsc_reg++);
+ fprintf(stdout, "\n");
+
+ return 0;
+}
+
--
1.7.10.4
^ permalink raw reply related
* Re: [patch net-next 0/4] net: allow to change carrier from userspace
From: Jiri Pirko @ 2012-12-18 9:31 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, davem, edumazet, bhutchings, mirqus, greearb, fbl
In-Reply-To: <20121217224957.70775f99@nehalam.linuxnetplumber.net>
Tue, Dec 18, 2012 at 07:49:57AM CET, shemminger@vyatta.com wrote:
>On Sun, 16 Dec 2012 11:54:51 +0100
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>>
>> I see that the patchset is in state "Rejected" in patchwork.
>> Stephen convinced me for a moment that the problem can be handled by operstate.
>> As it turned out (in last 3-4 emails in thread) operstate use would not
>> be an option.
>>
>> So how should I proceed? Should I repost the patchset? Anyone has any other
>> comments?
>>
>> thanks.
>
>Don't take my comments so far as negative. Devices to need to be more controllable
>from userspace. But I have concerns about introducing a new way to change state causing
>more races. For example, changing carrier state should cause netlink events to fire and
>these should post to routing daemons etc. Also, what happens if some confused developer
>mixes operstate and direct carrier control.
I do not think that the race you are describing is of any concern. The
same can happen now for any device. My patchset only adds a possibility
for "soft devices" to change the carrier as well.
Developer will not be likely confused. As the possibility of carrier
change from userspace will be limited to small set of devices, for other
devices the attempt will lead to -EOPNOTSUPP (in contrast with operstate
which is available for all devices).
I can add a comments/notes to code and operstates.txt stating the
purpose of this iface.
>
>The root cause of all this confusion is that their are three ways of expressing
>the same state, and they are controlled through different paths:
> a. Old BSD style flag bit IFF_RUNNING
> b. LINK_STATE bit in kernel (netif_carrier_ok)
> c. RFC2863 operational state
I do not think so. Yes, for a) and c), these are strictly connected,
expressing the same thing. But b) is not the same. It's on lower level
than a) and c). What b) can be compared to is IFF_LOWER_UP.
>
>The operstate stuff is the most complete, but is the weakest in implementation:
> a. kernel drivers check netif_carrier_ok when they should be using netif_dormant
> (bridge is one example). But what will break if this changes?
I agree, that should be changed.
> b. lower device state is not tracked correctly by tunnels and a few other layered devices
> c. dormant from kernel space was never used by much.
>
>The good news is that the old BSD style IFF_RUNNING bit is the most commonly
>used bit by applications and it works correctly in either carrier or operstate mode.
That is indeed a good thing.
^ permalink raw reply
* RE: [PATCH v2] netlink: align attributes on 64-bits
From: David Laight @ 2012-12-18 9:19 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: bhutchings, tgraf, netdev, davem
In-Reply-To: <50CF57DC.5050804@6wind.com>
> Le 17/12/2012 18:06, David Laight a écrit :
> >> int nla_put(struct sk_buff *skb, int attrtype, int attrlen, const void *data)
> >> {
> >> - if (unlikely(skb_tailroom(skb) < nla_total_size(attrlen)))
> >> + int align = IS_ALIGNED((unsigned long)skb_tail_pointer(skb), 8) ? 0 : 4;
> >
> > I've just realised where you are adding this!
> > You only want to add pad if the attribute is a single 64bit item,
> > not whenever the destination is misaligned.
> As said in the commit log, I want to align all attributes. An attribute can be
> like this:
>
> struct foo {
> __u32 bar1;
> __u32 bar2;
> __u64 bar3;
> }
>
> nla_put() don't know what is contained in the attribute.
Put there is no need to 8-byte align something whose size isn't a
multiple of 8 bytes.
> > ...
> >> + if (align) {
> >> + /* Goal is to add an attribute with size 4. We know that
> >> + * NLA_HDRLEN is 4, hence payload is 0.
> >> + */
> >> + __nla_reserve(skb, 0, 0);
> >
> > One of those zeros should be 'align - 4', then the comment
> > can be more descriptive.
> I thought if you were to research why we use 0, you would know that the first 0
> is the type and the second is the payload size...
I can tell that one is the type and the other the size, you've
implied that the 'type+size' actually total 4 bytes.
I don't need to find out which is which!
Now you've told me I'd have written:
_nla_reserve(skb, 0, align - NLA_HDRLEN);
The compiler could well have tracked the value - so know it is 4.
OTOH you might want to generate the size of 'align' without
using a conditional.
David
^ permalink raw reply
* Re: [PATCH 4/4] FEC: Add time stamping code and a PTP hardware clock
From: Frank Li @ 2012-12-18 8:51 UTC (permalink / raw)
To: Richard Cochran
Cc: Sascha Hauer, Shawn Guo, Frank Li, lznua, linux-arm-kernel,
netdev, davem
In-Reply-To: <20121218070420.GA2946@netboy.at.omicron.at>
>>
>> might be an option, but given that this patch seems to have bypassed any
>> review I feel more like reverting it.
>
> Instead of reverting, I suggest finding a solution (Frank) to let the
> code work when it can work and to prevent it when it cannot. This
> could be kconfig, DT, or run time probing of silicon revisions, but I
> don't have access to this hardware, and so I can't really say how to
> fix it.
I am traveling in this week. I will try to find out the solution after
come back office
in next week. But quick solution is shawn's patch, which resolve mx3
and mx5 problem at least in short term.
>
> Just for the record, I did in fact review this patch, and I commented
> on exactly this point. Frank said we would address this point, and he
> did so. Not knowing the imx family very well, I took his word for it.
> After all, Frank has a Freescale address, and I would expect a
> Freescale employee to know how to provide the right fix.
>
> Thanks,
> Richard
^ permalink raw reply
* [PATCH net-next] xfrm: removes a superfluous check and add a statistic
From: roy.qing.li @ 2012-12-18 8:39 UTC (permalink / raw)
To: netdev
From: Li RongQing <roy.qing.li@gmail.com>
Remove the check if x->km.state equal to XFRM_STATE_VALID in
xfrm_state_check_expire(), which will be done before call
xfrm_state_check_expire().
add a LINUX_MIB_XFRMOUTSTATEINVALID statistic to record the
outbound error due to invalid xfrm state.
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
include/uapi/linux/snmp.h | 1 +
net/xfrm/xfrm_output.c | 6 ++++++
net/xfrm/xfrm_proc.c | 1 +
net/xfrm/xfrm_state.c | 3 ---
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index fdfba23..93b24ce 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -274,6 +274,7 @@ enum
LINUX_MIB_XFRMOUTSTATEMODEERROR, /* XfrmOutStateModeError */
LINUX_MIB_XFRMOUTSTATESEQERROR, /* XfrmOutStateSeqError */
LINUX_MIB_XFRMOUTSTATEEXPIRED, /* XfrmOutStateExpired */
+ LINUX_MIB_XFRMOUTSTATEINVALID, /* XfrmOutStateInvalid */
LINUX_MIB_XFRMOUTPOLBLOCK, /* XfrmOutPolBlock */
LINUX_MIB_XFRMOUTPOLDEAD, /* XfrmOutPolDead */
LINUX_MIB_XFRMOUTPOLERROR, /* XfrmOutPolError */
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 95a338c..3670526 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -61,6 +61,12 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
}
spin_lock_bh(&x->lock);
+
+ if (unlikely(x->km.state != XFRM_STATE_VALID)) {
+ XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTSTATEINVALID);
+ goto error_nolock;
+ }
+
err = xfrm_state_check_expire(x);
if (err) {
XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTSTATEEXPIRED);
diff --git a/net/xfrm/xfrm_proc.c b/net/xfrm/xfrm_proc.c
index d0a1af8..e4cd441 100644
--- a/net/xfrm/xfrm_proc.c
+++ b/net/xfrm/xfrm_proc.c
@@ -39,6 +39,7 @@ static const struct snmp_mib xfrm_mib_list[] = {
SNMP_MIB_ITEM("XfrmOutStateModeError", LINUX_MIB_XFRMOUTSTATEMODEERROR),
SNMP_MIB_ITEM("XfrmOutStateSeqError", LINUX_MIB_XFRMOUTSTATESEQERROR),
SNMP_MIB_ITEM("XfrmOutStateExpired", LINUX_MIB_XFRMOUTSTATEEXPIRED),
+ SNMP_MIB_ITEM("XfrmOutStateInvalid", LINUX_MIB_XFRMOUTSTATEINVALID),
SNMP_MIB_ITEM("XfrmOutPolBlock", LINUX_MIB_XFRMOUTPOLBLOCK),
SNMP_MIB_ITEM("XfrmOutPolDead", LINUX_MIB_XFRMOUTPOLDEAD),
SNMP_MIB_ITEM("XfrmOutPolError", LINUX_MIB_XFRMOUTPOLERROR),
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 3459692..05db236 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1370,9 +1370,6 @@ int xfrm_state_check_expire(struct xfrm_state *x)
if (!x->curlft.use_time)
x->curlft.use_time = get_seconds();
- if (x->km.state != XFRM_STATE_VALID)
- return -EINVAL;
-
if (x->curlft.bytes >= x->lft.hard_byte_limit ||
x->curlft.packets >= x->lft.hard_packet_limit) {
x->km.state = XFRM_STATE_EXPIRED;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH 4/4] FEC: Add time stamping code and a PTP hardware clock
From: Richard Cochran @ 2012-12-18 7:04 UTC (permalink / raw)
To: Sascha Hauer
Cc: Frank Li, netdev, lznua, Frank Li, Shawn Guo, davem,
linux-arm-kernel
In-Reply-To: <20121217200232.GS26326@pengutronix.de>
On Mon, Dec 17, 2012 at 09:02:32PM +0100, Sascha Hauer wrote:
> This leaves an option in the tree which can be used to break FEC on
> i.MX3/5.
>
> depends on !SOC_IMX31 && !SOC_IMX35 && !SOC_IMX5
>
> might be an option, but given that this patch seems to have bypassed any
> review I feel more like reverting it.
Instead of reverting, I suggest finding a solution (Frank) to let the
code work when it can work and to prevent it when it cannot. This
could be kconfig, DT, or run time probing of silicon revisions, but I
don't have access to this hardware, and so I can't really say how to
fix it.
Just for the record, I did in fact review this patch, and I commented
on exactly this point. Frank said we would address this point, and he
did so. Not knowing the imx family very well, I took his word for it.
After all, Frank has a Freescale address, and I would expect a
Freescale employee to know how to provide the right fix.
Thanks,
Richard
^ permalink raw reply
* Re: [patch net-next 0/4] net: allow to change carrier from userspace
From: Stephen Hemminger @ 2012-12-18 6:49 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, edumazet, bhutchings, mirqus, greearb, fbl
In-Reply-To: <20121216105451.GA1546@minipsycho.orion>
On Sun, 16 Dec 2012 11:54:51 +0100
Jiri Pirko <jiri@resnulli.us> wrote:
>
> I see that the patchset is in state "Rejected" in patchwork.
> Stephen convinced me for a moment that the problem can be handled by operstate.
> As it turned out (in last 3-4 emails in thread) operstate use would not
> be an option.
>
> So how should I proceed? Should I repost the patchset? Anyone has any other
> comments?
>
> thanks.
Don't take my comments so far as negative. Devices to need to be more controllable
from userspace. But I have concerns about introducing a new way to change state causing
more races. For example, changing carrier state should cause netlink events to fire and
these should post to routing daemons etc. Also, what happens if some confused developer
mixes operstate and direct carrier control.
The root cause of all this confusion is that their are three ways of expressing
the same state, and they are controlled through different paths:
a. Old BSD style flag bit IFF_RUNNING
b. LINK_STATE bit in kernel (netif_carrier_ok)
c. RFC2863 operational state
The operstate stuff is the most complete, but is the weakest in implementation:
a. kernel drivers check netif_carrier_ok when they should be using netif_dormant
(bridge is one example). But what will break if this changes?
b. lower device state is not tracked correctly by tunnels and a few other layered devices
c. dormant from kernel space was never used by much.
The good news is that the old BSD style IFF_RUNNING bit is the most commonly
used bit by applications and it works correctly in either carrier or operstate mode.
^ permalink raw reply
* [PATCH 2/2] be2net: fix wrong frag_idx reported by RX CQ
From: Sathya Perla @ 2012-12-18 5:38 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
In-Reply-To: <1355809131-8924-1-git-send-email-sathya.perla@emulex.com>
The RX CQ can report completions with invalid frag_idx when the RXQ that
was *previously* using it, was not cleaned up properly. This hits
a BUG_ON() in be2net.
When completion coalescing is enabled on a CQ, an explicit CQ-notify
(with rearm) is needed for each compl, to flush partially coalesced CQ
entries that are pending DMA.
In be_close(), this fix now notifies CQ for each compl, waits explicitly
for the flush compl to arrive and complains if it doesn't arrive.
Also renaming be_crit_error() to be_hw_error() as it's the more
appropriate name and to convey that we don't wait for the flush compl
only when a HW error has occurred.
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/emulex/benet/be.h | 2 +-
drivers/net/ethernet/emulex/benet/be_main.c | 38 ++++++++++++++++++++++----
2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index abf26c7..3bc1912 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -616,7 +616,7 @@ static inline bool be_error(struct be_adapter *adapter)
return adapter->eeh_error || adapter->hw_error || adapter->fw_timeout;
}
-static inline bool be_crit_error(struct be_adapter *adapter)
+static inline bool be_hw_error(struct be_adapter *adapter)
{
return adapter->eeh_error || adapter->hw_error;
}
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index bf50e73..9dca22b 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1689,15 +1689,41 @@ static void be_rx_cq_clean(struct be_rx_obj *rxo)
struct be_queue_info *rxq = &rxo->q;
struct be_queue_info *rx_cq = &rxo->cq;
struct be_rx_compl_info *rxcp;
+ struct be_adapter *adapter = rxo->adapter;
+ int flush_wait = 0;
u16 tail;
- /* First cleanup pending rx completions */
- while ((rxcp = be_rx_compl_get(rxo)) != NULL) {
- be_rx_compl_discard(rxo, rxcp);
- be_cq_notify(rxo->adapter, rx_cq->id, false, 1);
+ /* Consume pending rx completions.
+ * Wait for the flush completion (identified by zero num_rcvd)
+ * to arrive. Notify CQ even when there are no more CQ entries
+ * for HW to flush partially coalesced CQ entries.
+ * In Lancer, there is no need to wait for flush compl.
+ */
+ for (;;) {
+ rxcp = be_rx_compl_get(rxo);
+ if (rxcp == NULL) {
+ if (lancer_chip(adapter))
+ break;
+
+ if (flush_wait++ > 10 || be_hw_error(adapter)) {
+ dev_warn(&adapter->pdev->dev,
+ "did not receive flush compl\n");
+ break;
+ }
+ be_cq_notify(adapter, rx_cq->id, true, 0);
+ mdelay(1);
+ } else {
+ be_rx_compl_discard(rxo, rxcp);
+ be_cq_notify(adapter, rx_cq->id, true, 1);
+ if (rxcp->num_rcvd == 0)
+ break;
+ }
}
- /* Then free posted rx buffer that were not used */
+ /* After cleanup, leave the CQ in unarmed state */
+ be_cq_notify(adapter, rx_cq->id, false, 0);
+
+ /* Then free posted rx buffers that were not used */
tail = (rxq->head + rxq->len - atomic_read(&rxq->used)) % rxq->len;
for (; atomic_read(&rxq->used) > 0; index_inc(&tail, rxq->len)) {
page_info = get_rx_page_info(rxo, tail);
@@ -2157,7 +2183,7 @@ void be_detect_error(struct be_adapter *adapter)
u32 sliport_status = 0, sliport_err1 = 0, sliport_err2 = 0;
u32 i;
- if (be_crit_error(adapter))
+ if (be_hw_error(adapter))
return;
if (lancer_chip(adapter)) {
--
1.7.1
^ permalink raw reply related
* [PATCH 1/2] be2net: fix be_close() to ensure all events are ack'ed
From: Sathya Perla @ 2012-12-18 5:38 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
In be_close(), be_eq_clean() must be called after all RX/TX/MCC queues
have been cleaned to ensure that any events caused while cleaning up
completions are notified/acked. Not clearing all events can cause
upredictable behaviour when RX rings are re-created in the subsequent
be_open().
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_cmds.c | 5 +++++
drivers/net/ethernet/emulex/benet/be_main.c | 21 ++++++++++++---------
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index f2875aa..8a250c3 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -298,7 +298,12 @@ void be_async_mcc_enable(struct be_adapter *adapter)
void be_async_mcc_disable(struct be_adapter *adapter)
{
+ spin_lock_bh(&adapter->mcc_cq_lock);
+
adapter->mcc_obj.rearm_cq = false;
+ be_cq_notify(adapter, adapter->mcc_obj.cq.id, false, 0);
+
+ spin_unlock_bh(&adapter->mcc_cq_lock);
}
int be_process_mcc(struct be_adapter *adapter)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index f95612b..bf50e73 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -2398,13 +2398,22 @@ static int be_close(struct net_device *netdev)
be_roce_dev_close(adapter);
- be_async_mcc_disable(adapter);
-
if (!lancer_chip(adapter))
be_intr_set(adapter, false);
- for_all_evt_queues(adapter, eqo, i) {
+ for_all_evt_queues(adapter, eqo, i)
napi_disable(&eqo->napi);
+
+ be_async_mcc_disable(adapter);
+
+ /* Wait for all pending tx completions to arrive so that
+ * all tx skbs are freed.
+ */
+ be_tx_compl_clean(adapter);
+
+ be_rx_qs_destroy(adapter);
+
+ for_all_evt_queues(adapter, eqo, i) {
if (msix_enabled(adapter))
synchronize_irq(be_msix_vec_get(adapter, eqo));
else
@@ -2414,12 +2423,6 @@ static int be_close(struct net_device *netdev)
be_irq_unregister(adapter);
- /* Wait for all pending tx completions to arrive so that
- * all tx skbs are freed.
- */
- be_tx_compl_clean(adapter);
-
- be_rx_qs_destroy(adapter);
return 0;
}
--
1.7.1
^ permalink raw reply related
* Re: [RFC][PATCH] ipv6 multicast forwarding: Remove threshold checking and some trivial bugs
From: Ang Way Chuang @ 2012-12-18 5:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev, yoshfuji
In-Reply-To: <20121217.210333.1952508082741483861.davem@davemloft.net>
Oops, sorry. You're right. I am not very confident with this modification. It may break some multicast
routing daemon. Let's drop this for now.
On 18/12/2012 13:03, David Miller wrote:
> From: Ang Way Chuang <wcang@sfc.wide.ad.jp>
> Date: Tue, 18 Dec 2012 12:57:11 +0800
>
>> This patch fixes trivial bugs for IPv6 multicast forwarding code and remove
>> threshold checking for multicast forwarding cache.
>>
>> 1. Threshold checking in IPv6 multicast forwarding cache (MFC) was not properly implemented.
>> syscall to setsockopt(... MRT6_ADD_MIF,...) doesn't affect the TTL because it is never used.
>> In fact, all MFC will always have ttl of 1 as set by ip6mr_mfc_add. From my limited knowledge of
>> multicast routing, threshold setting on interface is only used by DVMRP which doesn't support
>> IPv6. FreeBSD's struct mif6ctl doesn't have vifc_threshold. This patch removes the ttl cruft
>> within kernel. Userspace ABI for backward compatibility. Can someone knowledgable in multicast
>> routing please verify whether my understanding is correct?
>> 2. Don't allow addition of MFC with non-existent multicast interface index.
>> 3. Don't allow addition of MFC where incoming interface is part of oif list. This does not make
>> sense. Why would we want to send a multicast back to the interface where it originates from.
>> 4. setsockopt(....MRT6_ADD_MIF, ) allows a "physical" interface to be registered as multicast
>> interface multiple times. This doesn't make sense. Don't allow registration duplicate
>> registration of the same "physical" interface.
>>
>> This patch has been tested, albeit minimally using a simple program. Is this patch okay for
>> inclusion? Will sign off if it is okay.
>
> How about we don't mix together a set of bug fixes, with a semantic
> change (the removal of the threshold checking)?
>
> I also don't see what the point is of not signing off on this change
> when you submit it.
>
> If you delay the signoff until after review, you're just causing it to
> take longer to have your changes integrated. It also makes it look
> like you didn't believe fully in your change, so you probably should
> have sent it as an RFC and listed your doubts in the email instead.
>
> Overall I would rate this as an extremely poor patch submission,
> sorry.
> --
> 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
>
^ permalink raw reply
* Re: [RFC][PATCH] ipv6 multicast forwarding: Remove threshold checking and some trivial bugs
From: David Miller @ 2012-12-18 5:03 UTC (permalink / raw)
To: wcang; +Cc: netdev, yoshfuji
In-Reply-To: <50CFF7A7.1070508@sfc.wide.ad.jp>
From: Ang Way Chuang <wcang@sfc.wide.ad.jp>
Date: Tue, 18 Dec 2012 12:57:11 +0800
> This patch fixes trivial bugs for IPv6 multicast forwarding code and remove
> threshold checking for multicast forwarding cache.
>
> 1. Threshold checking in IPv6 multicast forwarding cache (MFC) was not properly implemented.
> syscall to setsockopt(... MRT6_ADD_MIF,...) doesn't affect the TTL because it is never used.
> In fact, all MFC will always have ttl of 1 as set by ip6mr_mfc_add. From my limited knowledge of
> multicast routing, threshold setting on interface is only used by DVMRP which doesn't support
> IPv6. FreeBSD's struct mif6ctl doesn't have vifc_threshold. This patch removes the ttl cruft
> within kernel. Userspace ABI for backward compatibility. Can someone knowledgable in multicast
> routing please verify whether my understanding is correct?
> 2. Don't allow addition of MFC with non-existent multicast interface index.
> 3. Don't allow addition of MFC where incoming interface is part of oif list. This does not make
> sense. Why would we want to send a multicast back to the interface where it originates from.
> 4. setsockopt(....MRT6_ADD_MIF, ) allows a "physical" interface to be registered as multicast
> interface multiple times. This doesn't make sense. Don't allow registration duplicate
> registration of the same "physical" interface.
>
> This patch has been tested, albeit minimally using a simple program. Is this patch okay for
> inclusion? Will sign off if it is okay.
How about we don't mix together a set of bug fixes, with a semantic
change (the removal of the threshold checking)?
I also don't see what the point is of not signing off on this change
when you submit it.
If you delay the signoff until after review, you're just causing it to
take longer to have your changes integrated. It also makes it look
like you didn't believe fully in your change, so you probably should
have sent it as an RFC and listed your doubts in the email instead.
Overall I would rate this as an extremely poor patch submission,
sorry.
^ permalink raw reply
* [RFC][PATCH] ipv6 multicast forwarding: Remove threshold checking and some trivial bugs
From: Ang Way Chuang @ 2012-12-18 4:57 UTC (permalink / raw)
To: netdev; +Cc: yoshfuji
This patch fixes trivial bugs for IPv6 multicast forwarding code and remove
threshold checking for multicast forwarding cache.
1. Threshold checking in IPv6 multicast forwarding cache (MFC) was not properly implemented.
syscall to setsockopt(... MRT6_ADD_MIF,...) doesn't affect the TTL because it is never used.
In fact, all MFC will always have ttl of 1 as set by ip6mr_mfc_add. From my limited knowledge of
multicast routing, threshold setting on interface is only used by DVMRP which doesn't support
IPv6. FreeBSD's struct mif6ctl doesn't have vifc_threshold. This patch removes the ttl cruft
within kernel. Userspace ABI for backward compatibility. Can someone knowledgable in multicast
routing please verify whether my understanding is correct?
2. Don't allow addition of MFC with non-existent multicast interface index.
3. Don't allow addition of MFC where incoming interface is part of oif list. This does not make
sense. Why would we want to send a multicast back to the interface where it originates from.
4. setsockopt(....MRT6_ADD_MIF, ) allows a "physical" interface to be registered as multicast
interface multiple times. This doesn't make sense. Don't allow registration duplicate
registration of the same "physical" interface.
This patch has been tested, albeit minimally using a simple program. Is this patch okay for
inclusion? Will sign off if it is okay.
---
include/linux/mroute6.h | 3 +-
net/ipv6/ip6mr.c | 79 +++++++++++++++++++++++++----------------------
2 files changed, 43 insertions(+), 39 deletions(-)
diff --git a/include/linux/mroute6.h b/include/linux/mroute6.h
index a223561..88a79d8 100644
--- a/include/linux/mroute6.h
+++ b/include/linux/mroute6.h
@@ -66,7 +66,6 @@ struct mif_device {
unsigned long bytes_in,bytes_out;
unsigned long pkt_in,pkt_out; /* Statistics */
unsigned long rate_limit; /* Traffic shaping (NI) */
- unsigned char threshold; /* TTL threshold */
unsigned short flags; /* Control flags */
int link; /* Physical interface index */
};
@@ -92,7 +91,7 @@ struct mfc6_cache {
unsigned long bytes;
unsigned long pkt;
unsigned long wrong_if;
- unsigned char ttls[MAXMIFS]; /* TTL thresholds */
+ struct if_set mf6c_ifset; /* Where it is going */
} res;
} mfc_un;
};
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 26dcdec..0a12fe4 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -122,6 +122,7 @@ static int ip6mr_rtm_dumproute(struct sk_buff *skb,
struct netlink_callback *cb);
static void mroute_clean_tables(struct mr6_table *mrt);
static void ipmr_expire_process(unsigned long arg);
+static int ip6mr_find_vif(struct mr6_table *mrt, struct net_device *dev);
#ifdef CONFIG_IPV6_MROUTE_MULTIPLE_TABLES
#define ip6mr_for_each_table(mrt, net) \
@@ -574,10 +575,10 @@ static int ipmr_mfc_seq_show(struct seq_file *seq, void *v)
for (n = mfc->mfc_un.res.minvif;
n < mfc->mfc_un.res.maxvif; n++) {
if (MIF_EXISTS(mrt, n) &&
- mfc->mfc_un.res.ttls[n] < 255)
+ IF_ISSET(n, &mfc->mfc_un.res.mf6c_ifset))
seq_printf(seq,
" %2d:%-3d",
- n, mfc->mfc_un.res.ttls[n]);
+ n, 1);
}
} else {
/* unresolved mfc_caches don't contain
@@ -895,28 +896,6 @@ static void ipmr_expire_process(unsigned long arg)
spin_unlock(&mfc_unres_lock);
}
-/* Fill oifs list. It is called under write locked mrt_lock. */
-
-static void ip6mr_update_thresholds(struct mr6_table *mrt, struct mfc6_cache *cache,
- unsigned char *ttls)
-{
- int vifi;
-
- cache->mfc_un.res.minvif = MAXMIFS;
- cache->mfc_un.res.maxvif = 0;
- memset(cache->mfc_un.res.ttls, 255, MAXMIFS);
-
- for (vifi = 0; vifi < mrt->maxvif; vifi++) {
- if (MIF_EXISTS(mrt, vifi) &&
- ttls[vifi] && ttls[vifi] < 255) {
- cache->mfc_un.res.ttls[vifi] = ttls[vifi];
- if (cache->mfc_un.res.minvif > vifi)
- cache->mfc_un.res.minvif = vifi;
- if (cache->mfc_un.res.maxvif <= vifi)
- cache->mfc_un.res.maxvif = vifi + 1;
- }
- }
-}
static int mif6_add(struct net *net, struct mr6_table *mrt,
struct mif6ctl *vifc, int mrtsock)
@@ -955,6 +934,12 @@ static int mif6_add(struct net *net, struct mr6_table *mrt,
dev = dev_get_by_index(net, vifc->mif6c_pifi);
if (!dev)
return -EADDRNOTAVAIL;
+
+ if (ip6mr_find_vif(mrt, dev) >= 0) {
+ dev_put(dev);
+ return -EADDRINUSE;
+ }
+
err = dev_set_allmulti(dev, 1);
if (err) {
dev_put(dev);
@@ -980,7 +965,6 @@ static int mif6_add(struct net *net, struct mr6_table *mrt,
v->flags = vifc->mif6c_flags;
if (!mrtsock)
v->flags |= VIFF_STATIC;
- v->threshold = vifc->vifc_threshold;
v->bytes_in = 0;
v->bytes_out = 0;
v->pkt_in = 0;
@@ -1393,22 +1377,37 @@ void ip6_mr_cleanup(void)
static int ip6mr_mfc_add(struct net *net, struct mr6_table *mrt,
struct mf6cctl *mfc, int mrtsock)
{
+ int minvif = MAXMIFS;
+ int maxvif = 0;
bool found = false;
int line;
struct mfc6_cache *uc, *c;
- unsigned char ttls[MAXMIFS];
int i;
- if (mfc->mf6cc_parent >= MAXMIFS)
+ if (mfc->mf6cc_parent >= MAXMIFS || !MIF_EXISTS(mrt, mfc->mf6cc_parent))
return -ENFILE;
- memset(ttls, 255, MAXMIFS);
- for (i = 0; i < MAXMIFS; i++) {
- if (IF_ISSET(i, &mfc->mf6cc_ifset))
- ttls[i] = 1;
+ /* incoming interface should not be part of outgoing interfaces, doing so
+ * will cause duplicate
+ */
+ if (IF_ISSET(mfc->mf6cc_parent, &mfc->mf6cc_ifset))
+ return -EINVAL;
+ for (i = 0; i < MAXMIFS; i++) {
+ if (IF_ISSET(i, &mfc->mf6cc_ifset)) {
+ if (!MIF_EXISTS(mrt, i))
+ return -ENFILE;
+
+ if (minvif > i)
+ minvif = i;
+ if (maxvif <= i)
+ maxvif = i + 1;
+ }
}
+ if (maxvif <= minvif) /* mf6cc_ifset is basically empty */
+ return -EINVAL;
+
line = MFC6_HASH(&mfc->mf6cc_mcastgrp.sin6_addr, &mfc->mf6cc_origin.sin6_addr);
list_for_each_entry(c, &mrt->mfc6_cache_array[line], list) {
@@ -1422,7 +1421,10 @@ static int ip6mr_mfc_add(struct net *net, struct mr6_table *mrt,
if (found) {
write_lock_bh(&mrt_lock);
c->mf6c_parent = mfc->mf6cc_parent;
- ip6mr_update_thresholds(mrt, c, ttls);
+ c->mfc_un.res.mf6c_ifset = mfc->mf6cc_ifset;
+ c->mfc_un.res.minvif = minvif;
+ c->mfc_un.res.maxvif = maxvif;
+
if (!mrtsock)
c->mfc_flags |= MFC_STATIC;
write_unlock_bh(&mrt_lock);
@@ -1440,7 +1442,10 @@ static int ip6mr_mfc_add(struct net *net, struct mr6_table *mrt,
c->mf6c_origin = mfc->mf6cc_origin.sin6_addr;
c->mf6c_mcastgrp = mfc->mf6cc_mcastgrp.sin6_addr;
c->mf6c_parent = mfc->mf6cc_parent;
- ip6mr_update_thresholds(mrt, c, ttls);
+ c->mfc_un.res.mf6c_ifset = mfc->mf6cc_ifset;
+ c->mfc_un.res.minvif = minvif;
+ c->mfc_un.res.maxvif = maxvif;
+
if (!mrtsock)
c->mfc_flags |= MFC_STATIC;
@@ -2036,7 +2041,7 @@ static int ip6_mr_forward(struct net *net, struct mr6_table *mrt,
large chunk of pimd to kernel. Ough... --ANK
*/
(mrt->mroute_do_pim ||
- cache->mfc_un.res.ttls[true_vifi] < 255) &&
+ IF_ISSET(true_vifi, &cache->mfc_un.res.mf6c_ifset)) &&
time_after(jiffies,
cache->mfc_un.res.last_assert + MFC_ASSERT_THRESH)) {
cache->mfc_un.res.last_assert = jiffies;
@@ -2052,7 +2057,7 @@ static int ip6_mr_forward(struct net *net, struct mr6_table *mrt,
* Forward the frame
*/
for (ct = cache->mfc_un.res.maxvif - 1; ct >= cache->mfc_un.res.minvif; ct--) {
- if (ipv6_hdr(skb)->hop_limit > cache->mfc_un.res.ttls[ct]) {
+ if (IF_ISSET(ct, &cache->mfc_un.res.mf6c_ifset)) {
if (psend != -1) {
struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
if (skb2)
@@ -2143,7 +2148,7 @@ static int __ip6mr_fill_mroute(struct mr6_table *mrt, struct sk_buff *skb,
return -EMSGSIZE;
for (ct = c->mfc_un.res.minvif; ct < c->mfc_un.res.maxvif; ct++) {
- if (MIF_EXISTS(mrt, ct) && c->mfc_un.res.ttls[ct] < 255) {
+ if (MIF_EXISTS(mrt, ct) && IF_ISSET(ct, &c->mfc_un.res.mf6c_ifset)) {
nhp = nla_reserve_nohdr(skb, sizeof(*nhp));
if (nhp == NULL) {
nla_nest_cancel(skb, mp_attr);
@@ -2151,7 +2156,7 @@ static int __ip6mr_fill_mroute(struct mr6_table *mrt, struct sk_buff *skb,
}
nhp->rtnh_flags = 0;
- nhp->rtnh_hops = c->mfc_un.res.ttls[ct];
+ nhp->rtnh_hops = 1; /* this is broken as IPv6 does not use TTL threshold */
nhp->rtnh_ifindex = mrt->vif6_table[ct].dev->ifindex;
nhp->rtnh_len = sizeof(*nhp);
}
^ permalink raw reply related
* Re: [PATCH 2/2 v2] cdc_ether: cleanup: use USB_DEVICE_AND_INTERFACE_INFO for Novatel 551/E362
From: David Miller @ 2012-12-18 4:52 UTC (permalink / raw)
To: dcbw; +Cc: bjorn, aleksander, netdev
In-Reply-To: <1355768386.1424.52.camel@dcbw.foobar.com>
From: Dan Williams <dcbw@redhat.com>
Date: Mon, 17 Dec 2012 12:19:46 -0600
> Signed-off-by: Dan Williams <dcbw@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2 v2] qmi_wwan/cdc_ether: add Dell Wireless 5800 (Novatel E362) USB IDs
From: David Miller @ 2012-12-18 4:52 UTC (permalink / raw)
To: dcbw; +Cc: bjorn, aleksander, netdev
In-Reply-To: <1355768261.1424.50.camel@dcbw.foobar.com>
From: Dan Williams <dcbw@redhat.com>
Date: Mon, 17 Dec 2012 12:17:41 -0600
> Signed-off-by: Dan Williams <dcbw@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH] atm: use scnprintf() instead of sprintf()
From: David Miller @ 2012-12-18 4:52 UTC (permalink / raw)
To: chas; +Cc: netdev, gang.chen
In-Reply-To: <20121217110001.3eaf3ac5@thirdoffive.cmf.nrl.navy.mil>
From: chas williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Date: Mon, 17 Dec 2012 11:00:01 -0500
>
> As reported by Chen Gang <gang.chen@asianux.com>, we should ensure there
> is enough space when formatting the sysfs buffers.
>
> Signed-off-by: Chas Williams <chas@cmf.nrl.navy.mil>
Applied.
^ permalink raw reply
* Re: [PATCH] netlink: validate addr_len on bind
From: David Miller @ 2012-12-18 4:52 UTC (permalink / raw)
To: hannes; +Cc: netdev
In-Reply-To: <20121216014219.GB1528@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Sun, 16 Dec 2012 02:42:19 +0100
> Otherwise an out of bounds read could happen.
>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Applied.
^ permalink raw reply
* Re: [PATCH] netlink: change presentation of portid in procfs to unsigned
From: David Miller @ 2012-12-18 4:51 UTC (permalink / raw)
To: hannes; +Cc: netdev
In-Reply-To: <20121216010919.GA1528@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Sun, 16 Dec 2012 02:09:19 +0100
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Applied.
^ permalink raw reply
* Re: [PATCH] tuntap: fix sparse warning
From: David Miller @ 2012-12-18 4:49 UTC (permalink / raw)
To: jasowang; +Cc: netdev, linux-kernel, fengguang.wu
In-Reply-To: <1355799627-55529-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Tue, 18 Dec 2012 11:00:27 +0800
> Make tun_enable_queue() static to fix the sparse warning:
>
> drivers/net/tun.c:399:19: sparse: symbol 'tun_enable_queue' was not declared. Should it be static?
>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied, thanks.
^ permalink raw reply
* [PATCH v4] netfilter: nf_conntrack_sip: Handle Cisco 7941/7945 IP phones
From: Kevin Cernekee @ 2012-12-18 4:33 UTC (permalink / raw)
To: pablo
Cc: David Woodhouse, Eric Dumazet, Patrick McHardy, David S. Miller,
Alexey Kuznetsov, Pekka Savola (ipv6), James Morris,
Hideaki YOSHIFUJI, Gabor Juhos, netfilter-devel, netfilter,
coreteam, linux-kernel, netdev
Most SIP devices use a source port of 5060/udp on SIP requests, so the
response automatically comes back to port 5060:
phone_ip:5060 -> proxy_ip:5060 REGISTER
proxy_ip:5060 -> phone_ip:5060 100 Trying
The newer Cisco IP phones, however, use a randomly chosen high source
port for the SIP request but expect the response on port 5060:
phone_ip:49173 -> proxy_ip:5060 REGISTER
proxy_ip:5060 -> phone_ip:5060 100 Trying
Standard Linux NAT, with or without nf_nat_sip, will send the reply back
to port 49173, not 5060:
phone_ip:49173 -> proxy_ip:5060 REGISTER
proxy_ip:5060 -> phone_ip:49173 100 Trying
But the phone is not listening on 49173, so it will never see the reply.
This patch modifies nf_*_sip to work around this quirk by extracting
the SIP response port from the Via: header, iff the source IP in the
packet header matches the source IP in the SIP request.
Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Patrick McHardy <kaber@trash.net>
---
Baseline: git://1984.lsi.us.es/nf-next
v3->v4 changes:
Fix patch context and APIs to match the current Linux tree. These
changes are from OpenWRT (Gabor?) and David W.
v4 was tested with Cisco 7945 (high UDP destination port) and Snom m9
(normal "symmetric" UDP destination port), both on IPv4 only.
I've been running a recent OpenWRT port of this patch (Attitude Adjustment
release, 3.3 kernel) for ~2mo, with both phones as clients.
include/linux/netfilter/nf_conntrack_sip.h | 3 +++
net/netfilter/nf_conntrack_sip.c | 17 +++++++++++++++++
net/netfilter/nf_nat_sip.c | 27 ++++++++++++++++++++++++---
3 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/include/linux/netfilter/nf_conntrack_sip.h b/include/linux/netfilter/nf_conntrack_sip.h
index 387bdd0..ba7f571 100644
--- a/include/linux/netfilter/nf_conntrack_sip.h
+++ b/include/linux/netfilter/nf_conntrack_sip.h
@@ -4,12 +4,15 @@
#include <net/netfilter/nf_conntrack_expect.h>
+#include <linux/types.h>
+
#define SIP_PORT 5060
#define SIP_TIMEOUT 3600
struct nf_ct_sip_master {
unsigned int register_cseq;
unsigned int invite_cseq;
+ __be16 forced_dport;
};
enum sip_expectation_classes {
diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index df8f4f2..72a67bb 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1440,8 +1440,25 @@ static int process_sip_request(struct sk_buff *skb, unsigned int protoff,
{
enum ip_conntrack_info ctinfo;
struct nf_conn *ct = nf_ct_get(skb, &ctinfo);
+ struct nf_ct_sip_master *ct_sip_info = nfct_help_data(ct);
+ enum ip_conntrack_dir dir = CTINFO2DIR(ctinfo);
unsigned int matchoff, matchlen;
unsigned int cseq, i;
+ union nf_inet_addr addr;
+ __be16 port;
+
+ /* Many Cisco IP phones use a high source port for SIP requests, but
+ * listen for the response on port 5060. If we are the local
+ * router for one of these phones, save the port number from the
+ * Via: header so that nf_nat_sip can redirect the responses to
+ * the correct port.
+ */
+ if (ct_sip_parse_header_uri(ct, *dptr, NULL, *datalen,
+ SIP_HDR_VIA_UDP, NULL, &matchoff,
+ &matchlen, &addr, &port) > 0 &&
+ port != ct->tuplehash[dir].tuple.src.u.udp.port &&
+ nf_inet_addr_cmp(&addr, &ct->tuplehash[dir].tuple.src.u3))
+ ct_sip_info->forced_dport = port;
for (i = 0; i < ARRAY_SIZE(sip_handlers); i++) {
const struct sip_handler *handler;
diff --git a/net/netfilter/nf_nat_sip.c b/net/netfilter/nf_nat_sip.c
index 16303c7..5951146e 100644
--- a/net/netfilter/nf_nat_sip.c
+++ b/net/netfilter/nf_nat_sip.c
@@ -95,6 +95,7 @@ static int map_addr(struct sk_buff *skb, unsigned int protoff,
enum ip_conntrack_info ctinfo;
struct nf_conn *ct = nf_ct_get(skb, &ctinfo);
enum ip_conntrack_dir dir = CTINFO2DIR(ctinfo);
+ struct nf_ct_sip_master *ct_sip_info = nfct_help_data(ct);
char buffer[INET6_ADDRSTRLEN + sizeof("[]:nnnnn")];
unsigned int buflen;
union nf_inet_addr newaddr;
@@ -107,7 +108,8 @@ static int map_addr(struct sk_buff *skb, unsigned int protoff,
} else if (nf_inet_addr_cmp(&ct->tuplehash[dir].tuple.dst.u3, addr) &&
ct->tuplehash[dir].tuple.dst.u.udp.port == port) {
newaddr = ct->tuplehash[!dir].tuple.src.u3;
- newport = ct->tuplehash[!dir].tuple.src.u.udp.port;
+ newport = ct_sip_info->forced_dport ? :
+ ct->tuplehash[!dir].tuple.src.u.udp.port;
} else
return 1;
@@ -144,6 +146,7 @@ static unsigned int nf_nat_sip(struct sk_buff *skb, unsigned int protoff,
enum ip_conntrack_info ctinfo;
struct nf_conn *ct = nf_ct_get(skb, &ctinfo);
enum ip_conntrack_dir dir = CTINFO2DIR(ctinfo);
+ struct nf_ct_sip_master *ct_sip_info = nfct_help_data(ct);
unsigned int coff, matchoff, matchlen;
enum sip_header_types hdr;
union nf_inet_addr addr;
@@ -258,6 +261,21 @@ next:
!map_sip_addr(skb, protoff, dataoff, dptr, datalen, SIP_HDR_TO))
return NF_DROP;
+ /* Mangle destination port for Cisco phones, then fix up checksums */
+ if (dir == IP_CT_DIR_REPLY && ct_sip_info->forced_dport) {
+ struct udphdr *uh;
+
+ if (!skb_make_writable(skb, skb->len))
+ return NF_DROP;
+
+ uh = (void *)skb->data + protoff;
+ uh->dest = ct_sip_info->forced_dport;
+
+ if (!nf_nat_mangle_udp_packet(skb, ct, ctinfo, protoff,
+ 0, 0, NULL, 0))
+ return NF_DROP;
+ }
+
return NF_ACCEPT;
}
@@ -311,8 +329,10 @@ static unsigned int nf_nat_sip_expect(struct sk_buff *skb, unsigned int protoff,
enum ip_conntrack_info ctinfo;
struct nf_conn *ct = nf_ct_get(skb, &ctinfo);
enum ip_conntrack_dir dir = CTINFO2DIR(ctinfo);
+ struct nf_ct_sip_master *ct_sip_info = nfct_help_data(ct);
union nf_inet_addr newaddr;
u_int16_t port;
+ __be16 srcport;
char buffer[INET6_ADDRSTRLEN + sizeof("[]:nnnnn")];
unsigned int buflen;
@@ -326,8 +346,9 @@ static unsigned int nf_nat_sip_expect(struct sk_buff *skb, unsigned int protoff,
/* If the signalling port matches the connection's source port in the
* original direction, try to use the destination port in the opposite
* direction. */
- if (exp->tuple.dst.u.udp.port ==
- ct->tuplehash[dir].tuple.src.u.udp.port)
+ srcport = ct_sip_info->forced_dport ? :
+ ct->tuplehash[dir].tuple.src.u.udp.port;
+ if (exp->tuple.dst.u.udp.port == srcport)
port = ntohs(ct->tuplehash[!dir].tuple.dst.u.udp.port);
else
port = ntohs(exp->tuple.dst.u.udp.port);
--
1.7.5.4
^ permalink raw reply related
* [PATCH] tuntap: fix sparse warning
From: Jason Wang @ 2012-12-18 3:00 UTC (permalink / raw)
To: davem, netdev, linux-kernel; +Cc: fengguang.wu, Jason Wang
Make tun_enable_queue() static to fix the sparse warning:
drivers/net/tun.c:399:19: sparse: symbol 'tun_enable_queue' was not declared. Should it be static?
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/tun.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 173acf5..504f7f1 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -396,7 +396,7 @@ static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
++tun->numdisabled;
}
-struct tun_struct *tun_enable_queue(struct tun_file *tfile)
+static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
{
struct tun_struct *tun = tfile->detached;
--
1.7.1
^ permalink raw reply related
* Re: Launch Time Support
From: Ulf samuelsson @ 2012-12-17 22:57 UTC (permalink / raw)
To: Vick, Matthew, netdev@vger.kernel.org
In-Reply-To: <06DFBC1E25D8024DB214DC7F41A3CD344897485E@ORSMSX101.amr.corp.intel.com>
17 dec 2012 kl. 22:44 skrev "Vick, Matthew" <matthew.vick@intel.com>:
>> -----Original Message-----
>> From: Ulf samuelsson [mailto:netdev@emagii.com]
>> Sent: Friday, December 14, 2012 11:35 PM
>> To: Vick, Matthew
>> Cc: netdev@vger.kernel.org
>> Subject: Re: Launch Time Support
>>
>>
>> 15 dec 2012 kl. 01:45 skrev "Vick, Matthew" <matthew.vick@intel.com>:
>>
>>>> -----Original Message-----
>>>> From: netdev-owner@vger.kernel.org [mailto:netdev-
>>>> owner@vger.kernel.org] On Behalf Of Ulf Samuelsson
>>>> Sent: Wednesday, December 12, 2012 5:04 PM
>>>> To: netdev@vger.kernel.org
>>>> Subject: RFC: Launch Time Support
>>>>
>>>> Hi, I am looking for some feedback on how to implement launchtime in
>>>> the kernel.
>>>>
>>>> I.E: You define WHEN you want to send a packet, and the driver will
>>>> store the packet in a buffer and will send it out on the net when
>> the
>>>> internal timestamp counter in the network controller reaches the
>>>> specified "launch time".
>>>>
>>>> Some Ethernet controllers like the new Intel i210 support "launch
>>>> time",
>>>>
>>>> Support for launch time is desirable for any isochronous connection,
>>>> but I am currently interested in the NTP protocol to improve the
>>>> timing.
>>>>
>>>> Proposed Changes to the Kernel
>>>> ===========================================================
>>>> The launchtime support will be dependent on CONFIG_NET_LAUNCHTIME If
>>>> this is not set, then the kernel functionality is not changed.
>>>>
>>>> My current idea is to add a new bit to the "flags" field of
>>>> "socket.c:sendto"
>>>> #define MSG_LAUNCHTIME 0x?????
>>>>
>>>> struct msghdr gets an additional launchtime field.
>>>>
>>>> sendto will check if the flags parameter contains MSG_LAUNCHTIME.
>>>> If it does, then the first 64 bit longword of the packet (buff)
>>>> contains the launchtime.
>>>> The launchtime from the buffer is copied to the msghdr.launchtime
>>>> field, and the first 64 bits of the packet is then shaved off,
>> before
>>>> the address is written to the msghdr.
>>>>
>>>> Each network controller supporting launchtime needs to have an
>>>> alternative call to "send packet with launchtime" . This call adds
>>>> the launchtime parameter.
>>>> If launchtime is supported the exported "ops" includes the new call.
>>>>
>>>> The UDP/IP packet send will check the MSG_LAUNCHTIME and if set, it
>>>> will check if the "send packet with launchtime" call is available
>> for
>>>> the driver and if so call it, otherwise it will call the normal send
>>>> packet and thus ignore the launchtime.
>>>>
>>>> Before launchtime is used, the application should send an ioctl to
>>>> the driver, making sure that launchtime is configured, and only if
>>>> the driver ACKs , the application will use launchtime.
>>>>
>>>> (Possibly the "ops" field for "send packet with launchtime" should
>> be
>>>> NULL until that ioctl is complete. Comments?)
>>>>
>>>> To me, this seems to be transparent for all other network stacks so
>>>> protocols and drivers not supporting launchtime should still work.
>>>>
>>>> As far as I know, drivers do not support launch time today.
>>>> The Intel igb driver does not in the latest version on the intel web
>>>> site, There are some defines headers in the latest version defining
>>>> the registers but so far, the code is not using it.
>>>>
>>>> There is the linux_igb_avb project on sourceforge which allows use
>>>> of launch time for user space applications, but not as part of the
>> kernel.
>>>>
>>>> Maybe there is more work done somewhere else, but i am not aware of
>>>> this, so any links to such work is appreciated.
>>>>
>>>> There are some FPGA based PCIe boards that support launchtime
>> (Endace
>>>> DAG) using proprietary APIs.
>>>> Talked to some vendors providing TCP/IP offload engines for FPGA and
>>>> they do not support launchtime and liuke Endace use proprietary APIs
>>>> so they are only useable by custom programs. Normal networking
>>>> interfaces are not supported.
>>>>
>>>> Comment on above is appreciated.
>>>>
>>>> BACKGROUND
>>>> For those that do not know how the NTP protocol works:
>>>> ===================================================
>>>> The client sends an UDP packet to the NTP server using port 123 The
>>>> NTP client reads the current systime and puts that in the outgoing
>> packet.
>>>> There is a delay between the time the systime is read, and the time
>>>> the packet actually leaves the Ethernet controller adding jitter to
>>>> the NTP algorithm.
>>>>
>>>> When the server receives the packet, it can be timestamped in H/W
>> and
>>>> a CMSG is then created by the network stack containing that
>> timestamp
>>>> for use by the server NTP daemon.
>>>>
>>>> The server generates a reply, which needs to include the client
>>>> transmit time, the servers receive time, and the servers transmit
>> time.
>>>> Again, the transmit time needs to be written into the NTP packet,
>> and
>>>> then it needs to be processed through the network stack before it is
>>>> leaving the ethernet controller causing more jitter.
>>>>
>>>> If launch time is supported, then the client NTP daemon would simply
>>>> read the systime, add a constant delay to create the transmit
>>>> timestamp.
>>>> The delay needs to be sufficiently large to ensure that all
>>>> processing is done,
>>>>
>>>> The server will do something similar adding a constant to the server
>>>> receive timestamp to create the server transmit timestamp.
>>>> If both the client and the server uses H/W timestamping and launch
>>>> time, then the the jitter ideally is reduced to zero.
>>>>
>>>> TRANSMIT TIMESTAMPING
>>>> ========================
>>>> Support for TX timestamps in H/W is not really useful, since you
>> need
>>>> to provide the TX timestamp in the packet you measure on, so when
>> you
>>>> know the timestamp it is too late. Server to server NTP connections
>>>> support sending that timestamp in a new packet, but there is no such
>>>> support in client server communication.
>>>>
>>>> The i210 supports putting the timestamp inside the packet as it
>>>> leaves the Ethernet controller, but that means that you screw up the
>>>> UDP checksum, so the packet will be rejected by the receiving NTP
>> daemon.
>>>> In addition, the i210 timestamp measures seconds and nanoseconds
>>>> which is incompatible with the NTP timestamp which uses seconds and
>> a
>>>> 32 bit fraction of a second so that does not work either.
>>>>
>>>> Best Regards
>>>> Ulf Samuelsson
>>>> eMagii.
>>>
>>> Ulf,
>>>
>>> I have been looking into adding launch time support as part of
>> enabling some of the I210 functionality you have described (such as in
>> linux_igb_avb on SourceForge) upstream--less focused on NTP and more
>> focused on AVB, but launch time will be necessary for both. If you
>> would like, please feel free to contact me and I would love to work
>> with you on this.
>>>
>>> Reading your proposal, I'm a little confused by which systime you're
>> referring to. Do you mean on the host or on the NIC? In the case of
>> hardware timestamping today, in igb we set the SYSTIM registers to the
>> current system time, but that doesn't mean that the host clock and the
>> NIC clock stay synced. You would either need a mechanism such as PPS
>> (which igb does not implement today) to sync the host clock to the NIC
>> clock or have the NTP daemon account for the discrepancy. Off the top
>> of my head, I want to say modern PTP daemons (such as ptp4l) account
>> for the discrepancy in the daemon.
>>>
>>> Cheers,
>>> Matthew
>>
>> We live in luxury, having access to a Cesium Clock ;-) and we define
>> the time, beeing a top-level (Stratum 1) server.
>>
>> There are some I/Os on the i210 that can be used to interface to the
>> PPS.
>>
>> As for reading systime, it is done indirectly as you get the systime as
>> part of the NTP incoming packet. (It is timestamped at reception) and
>> add the constant to that value.
>>
>> Best Regards
>> Ulf Samuelsson
>
> So your proposal is to use a PPS interface (from some Stratum 1 server) to drive the clock on an I210 so you can use the I210's launch time mechanism to send packets at a certain time--is this correct? Or are you talking more about a software launch time solution?
There are 4 I/O pins, and you can capture the timestamp when they change value.
This can be used to calibrate the timestamp vs the 1 PPS clock.
It is far from ideal, but it might just work.
If you can clock the part from a 5 MHz clock from the Cesium clock, that would be even better.
>
> Forgive my ignorance, but what constant are you referring to in the receive path? Based on your first e-mail, you mention the constant should be added to the transmit path.
It is an arbitrary time constant you select in the range of 10s of milliseconds.
The only requirement is that the time from receiving the incoming packet,
to the time when the reply leaves the machine is always shorter than the time constant.
We have seen that this has been implemented on a 10 GbE FPGA card using 50 ms.
There is no requirement for short latency in NTP, only predictable latency.
>
> Also, how will you account for hardware discrepancies? For example, how far in the future you can schedule a packet will differ from hardware to hardware.
We don't have the problem, since it is for internal use, and we will use the same H/W
In all nodes. Possibly you want to be able to query the driver for your use.
Best Regards
Ulf Samuelsson
ulf@emagii.com
>
> Cheers,
> Matthew
> --
> 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
^ 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