* Re: [RFC PATCH 2/2] net: ethtool: Add capability to retrieve plug-in module EEPROM
From: Stuart Hodgson @ 2012-04-11 17:41 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, davem, linux-kernel
In-Reply-To: <1333390698.2623.47.camel@bwh-desktop.uk.solarflarecom.com>
On 02/04/12 19:18, Ben Hutchings wrote:
> On Tue, 2012-03-27 at 18:51 +0100, Stuart Hodgson wrote:
>> Implementation in sfc driver to return the plugin module eeprom
>>
>> Currently allows for SFP+ eeprom to be returned using the ethtool API.
>> This can be extended in future to handle different eeprom formats
>> and sizes.
>>
>> Signed-off-by: Stuart Hodgson<smhodgson@solarflare.com>
>> ---
>> drivers/net/ethernet/sfc/ethtool.c | 36 +++++++++++
>> drivers/net/ethernet/sfc/mcdi_phy.c | 105
>> +++++++++++++++++++++++++++++++++
>> drivers/net/ethernet/sfc/net_driver.h | 5 ++
>> 3 files changed, 146 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/sfc/ethtool.c
>> b/drivers/net/ethernet/sfc/ethtool.c
>> index f22f45f..e77895f 100644
>> --- a/drivers/net/ethernet/sfc/ethtool.c
>> +++ b/drivers/net/ethernet/sfc/ethtool.c
>> @@ -1108,6 +1108,40 @@ static int efx_ethtool_set_rxfh_indir(struct
>> net_device *net_dev,
>> return 0;
>> }
>>
>> +static int efx_ethtool_get_module_eeprom(struct net_device *net_dev,
>> + struct ethtool_eeprom *ee,
>> + u8 *data)
>> +{
>> + struct efx_nic *efx = netdev_priv(net_dev);
>> + int ret;
>> +
>> + if (!efx->phy_op ||
>> + !efx->phy_op->get_module_eeprom)
>
> No need to break that line.
>
> [...]
>> --- a/drivers/net/ethernet/sfc/mcdi_phy.c
>> +++ b/drivers/net/ethernet/sfc/mcdi_phy.c
>> @@ -304,6 +304,17 @@ static u32 mcdi_to_ethtool_media(u32 media)
>> }
>> }
>>
>> +static u32 mcdi_to_module_eeprom_len(u32 media)
>> +{
>> + switch (media) {
>> + case MC_CMD_MEDIA_SFP_PLUS:
>> + return SFF_8079_LEN;
>> + case MC_CMD_MEDIA_XFP:
>
> Why split out XFP if we're not going to treat it any differently?
>
>> + default:
>> + return 0;
>> + }
>> +}
>> +
>> static int efx_mcdi_phy_probe(struct efx_nic *efx)
>> {
>> struct efx_mcdi_phy_data *phy_data;
>> @@ -739,6 +750,98 @@ static const char *efx_mcdi_phy_test_name(struct
>> efx_nic *efx,
>> return NULL;
>> }
>>
>> +#define SFP_PAGE_SIZE 128
>> +#define NUM_PAGES 2
>
> SFP_NUM_PAGES?
>
>> +#define OFF_TO_BUFF(x) (x + MC_CMD_GET_PHY_MEDIA_INFO_OUT_DATA_OFST)
>
> This is not really worth defining for just the one use.
>
>> +static int efx_mcdi_phy_get_module_eeprom(struct efx_nic *efx,
>> + struct ethtool_eeprom *ee, u8 *data)
>> +{
>> + u8 outbuf[MC_CMD_GET_PHY_MEDIA_INFO_OUT_LENMAX];
>> + u8 inbuf[MC_CMD_GET_PHY_MEDIA_INFO_IN_LEN];
>> + size_t outlen;
>> + int rc;
>> + int payload_len;
>> + int copied = 0;
>> + int space_remaining = ee->len;
>> + int page;
>> + int page_off;
>> + int to_copy;
>
> None of payload_len..to_copy should be signed.
>
>> + u8 *user_data = data;
>> +
>> + if (!data || !ee)
>> + return -EINVAL;
>
> Neither of these is allowed to be NULL; don't bother checking.
>
>> + if (ee->offset> (SFP_PAGE_SIZE * NUM_PAGES)) {
>> + rc = -EINVAL;
>> + goto fail;
>> + }
>> +
>> + page_off = (ee->offset % SFP_PAGE_SIZE);
>> + page = (ee->offset> SFP_PAGE_SIZE) ? 1 : 0;
>
> Why not ee->offset / SFP_PAGE_SIZE?
>
>> + while (space_remaining&& (page< NUM_PAGES)) {
>> +
>> + MCDI_SET_DWORD(inbuf, GET_PHY_MEDIA_INFO_IN_PAGE, page);
>> +
>> + rc = efx_mcdi_rpc(efx, MC_CMD_GET_PHY_MEDIA_INFO,
>> + inbuf, sizeof(inbuf),
>> + outbuf, sizeof(outbuf),
>> +&outlen);
>> +
>> + if (rc)
>> + goto fail;
>> +
>> + /* Copy as much as we can into data */
>> + if (outlen< MC_CMD_GET_PHY_MEDIA_INFO_OUT_LENMIN ||
>> + outlen> MC_CMD_GET_PHY_MEDIA_INFO_OUT_LENMAX) {
>> + rc = -EIO;
>> + goto fail;
>> + }
>> +
>> + payload_len = MCDI_DWORD(outbuf,
>> + GET_PHY_MEDIA_INFO_OUT_DATALEN);
>> +
>> + to_copy = (space_remaining< payload_len) ?
>> + space_remaining : payload_len;
>> +
>> + to_copy -= page_off;
>
> page_off is the number of bytes we need to discard from payload_len, but
> we don't want do discard that from space_remaining. I think the last
> two statements should be changed to:
>
> payload_len -= page_off;
> to_copy = (space_remaining< payload_len) ?
> space_remaining : payload_len;
>
I am pretty sure that these two pieces of code are the same
>> + memcpy(user_data,
>> + (outbuf + OFF_TO_BUFF(page_off)),
>> + to_copy);
>> +
>> + space_remaining -= to_copy;
>> + user_data += to_copy;
>> + copied += to_copy;
>> + page_off = 0;
>> + page++;
>> + }
>> +
>> + ee->len = copied;
>> +
>> + return 0;
>> +fail:
>> + return rc;
>
> Nothing to clean up here, so you might as well return errors directly.
>
>> +}
>> +
>> +static int efx_mcdi_phy_get_module_info(struct efx_nic *efx,
>> + struct ethtool_modinfo *modinfo)
>> +{
>> + /* This will return a length of the eeprom
>> + * type of the module that was detected during the probe,
>> + * if not modules inserted then phy_data will be NULL */
>> + struct efx_mcdi_phy_data *phy_cfg;
>> +
>> + if (!efx || !efx->phy_data)
>> + return -EOPNOTSUPP;
>
> efx certainly can't be NULL here and I don't believe efx->phy_data can
> either. (None of the other MCDI PHY operations check it.)
>
>> + phy_cfg = efx->phy_data;
>> + modinfo->eeprom_len = mcdi_to_module_eeprom_len(phy_cfg->media);
>> + modinfo->type = SFF_8079;
> [...]
>
> I don't think this makes sense. If we're fixing the type as SFF_8079
> then why are we calling a function to get the length?
>
> Ben.
>
What about adding an mcdi_to_module_eeprom_type in the same manner
as mcdi_to_module_eeprom_len and the other mapping functions?
Stu
^ permalink raw reply
* Re: [net-next PATCH v1 1/7] net: add generic PF_BRIDGE:RTM_ FDB hooks
From: John Fastabend @ 2012-04-11 17:22 UTC (permalink / raw)
To: Ben Hutchings
Cc: roprabhu, mst, stephen.hemminger, davem, hadi, jeffrey.t.kirsher,
netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <1334160350.2552.2.camel@bwh-desktop.uk.solarflarecom.com>
On 4/11/2012 9:05 AM, Ben Hutchings wrote:
> On Wed, 2012-04-11 at 07:45 -0700, John Fastabend wrote:
>> On 4/10/2012 8:23 PM, Ben Hutchings wrote:
>>> On Mon, 2012-04-09 at 15:00 -0700, John Fastabend wrote:
>>> [...]
>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>> index 1f77540..05822e5 100644
>>>> --- a/include/linux/netdevice.h
>>>> +++ b/include/linux/netdevice.h
>>> [...]
>>>> @@ -905,6 +906,19 @@ struct netdev_fcoe_hbainfo {
>>>> * feature set might be less than what was returned by ndo_fix_features()).
>>>> * Must return >0 or -errno if it changed dev->features itself.
>>>> *
>>>> + * int (*ndo_fdb_add)(struct ndmsg *ndm, struct net_device *dev,
>>>> + * unsigned char *addr, u16 flags)
>>>> + * Adds an FDB entry to dev for addr. The ndmsg contains flags to indicate
>>>> + * if the dev->master FDB should be updated or the devices internal FDB.
>>>
>>> I don't think the second sentence is helpful, as rtnl_fdb_add() will
>>> take care of those flags.
>>>
>>>> + * int (*ndo_fdb_del)(struct ndmsg *ndm, struct net_device *dev,
>>>> + * unsigned char *addr)
>>>> + * Deletes the FDB entry from dev coresponding to addr. The ndmsg
>>>> + * contains flags to indicate if the dev->master FDB should be
>>>> + * updated or the devices internal FDB.
>>>
>>> Similarly here.
>>
>> agreed neither seem particularly helpful I'll remove them.
>>
>>>
>>>> + * int (*ndo_fdb_dump)(struct sk_buff *skb, struct netlink_callback *cb,
>>>> + * struct net_device *dev, int idx)
>>>> + * Used to add FDB entries to dump requests. Implementers should add
>>>> + * entries to skb and update idx with the number of entries.
>>>> */
>>> [...
>>>> --- a/net/core/rtnetlink.c
>>>> +++ b/net/core/rtnetlink.c
>>> [...]
>>>> +static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
>>>> +{
>>> [...]
>>>> + err = -EOPNOTSUPP;
>>>
>>> So if NTF_MASTER and NTF_SELF are both set, we can quietly fall back to
>>> just setting one FDB? Not sure that's really desirable though
>>>
>>
>> It makes it easier to keep an embedded agent and the sw bridge in
>> sync if setting both flags adds the entry to both the SW bridge and
>> embedded bridge.
>
> Yes, I agree with that.
>
> [...]
>>> Wonder what we should do on error here if we've already successfully
>>> called ndo_fdb_add on the master? Should we try to roll back the first
>>> addition?
>>>
>>
>> The problem with rolling back is the table is likely already updated and
>> traffic may already be being forwarded. So I think in this case the user
>> will have to query the device to learn what failed. It seems like the
>> simplest way to handle this. I think it is unwanted to have traffic being
>> forwarded one way for a short period of time then rolled back.
>>
>> The other idea I just had is we could clear the NTF_ bit in ndm_flags after
>> the successful add, del command. I believe the nlmsg gets sent back to the
>> user on error I would need to check on this.
> [...]
>
> That sounds like a good way of doing it, assuming there's no
> compatibility issue.
>
> Ben.
>
I don't see any compat issues. Current applications shouldn't be setting
any flags and shouldn't be expecting any flags in the ack.
Also I think I need to add some notify hooks to help management. The
bridge code has fdb_notify() so I'll add a similar rtnl_fdb_notify() hook
here. Caught this while implementing some user space daemon code.
Thanks! v2 coming soon.
^ permalink raw reply
* [PATCH 8/8] r6040: update copyright from 2007 to now
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
drivers/net/ethernet/rdc/r6040.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index 050cf59..4de7364 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -4,7 +4,7 @@
* Copyright (C) 2004 Sten Wang <sten.wang@rdc.com.tw>
* Copyright (C) 2007
* Daniel Gimpelevich <daniel@gimpelevich.san-francisco.ca.us>
- * Florian Fainelli <florian@openwrt.org>
+ * Copyright (C) 2007-2012 Florian Fainelli <florian@openwrt.org>
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
--
1.7.5.4
^ permalink raw reply related
* [PATCH 4/8] r6040: fix typo on stats update in tx path
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>
We are currently updating the rx fifo error counter in the tx path while
it should have been the tx fifo error counter, fix that.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
drivers/net/ethernet/rdc/r6040.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index 9ffbf6e..db33573 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -605,7 +605,7 @@ static void r6040_tx(struct net_device *dev)
err = ioread16(ioaddr + MLSR);
if (err & 0x0200)
- dev->stats.rx_fifo_errors++;
+ dev->stats.tx_fifo_errors++;
if (err & (0x2000 | 0x4000))
dev->stats.tx_carrier_errors++;
--
1.7.5.4
^ permalink raw reply related
* [PATCH 6/8] r6040: define and use MTPR transmit enable bit
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>
Define MTPR bit 0 of the register and use it where it is appropriate.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
drivers/net/ethernet/rdc/r6040.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index 1a365ae..afa4186 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -74,6 +74,7 @@
#define MT_ICR 0x0C /* TX interrupt control */
#define MR_ICR 0x10 /* RX interrupt control */
#define MTPR 0x14 /* TX poll command register */
+#define TM2TX 0x0001 /* Trigger MAC to transmit */
#define MR_BSR 0x18 /* RX buffer size */
#define MR_DCR 0x1A /* RX descriptor control */
#define MLSR 0x1C /* Last status */
@@ -420,7 +421,7 @@ static void r6040_init_mac_regs(struct net_device *dev)
/* Let TX poll the descriptors
* we may got called by r6040_tx_timeout which has left
* some unsent tx buffers */
- iowrite16(0x01, ioaddr + MTPR);
+ iowrite16(TM2TX, ioaddr + MTPR);
}
static void r6040_tx_timeout(struct net_device *dev)
@@ -844,7 +845,7 @@ static netdev_tx_t r6040_start_xmit(struct sk_buff *skb,
skb_tx_timestamp(skb);
/* Trigger the MAC to check the TX descriptor */
- iowrite16(0x01, ioaddr + MTPR);
+ iowrite16(TM2TX, ioaddr + MTPR);
lp->tx_insert_ptr = descptr->vndescp;
/* If no tx resource, stop */
--
1.7.5.4
^ permalink raw reply related
* [PATCH 7/8] r6040: define and use bits of register PHY_CC
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>
Define and use the bits of the PHY_CC (status change configuration) register.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
drivers/net/ethernet/rdc/r6040.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index afa4186..050cf59 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -128,6 +128,9 @@
#define MID_3M 0x82 /* MID3 Medium */
#define MID_3H 0x84 /* MID3 High */
#define PHY_CC 0x88 /* PHY status change configuration register */
+#define SCEN 0x8000 /* PHY status change enable */
+#define PHYAD_SHIFT 8 /* PHY address shift */
+#define TMRDIV_SHIFT 0 /* Timer divider shift */
#define PHY_ST 0x8A /* PHY status register */
#define MAC_SM 0xAC /* MAC status machine */
#define MAC_SM_RST 0x0002 /* MAC status machine reset */
@@ -1132,10 +1135,15 @@ static int __devinit r6040_init_one(struct pci_dev *pdev,
err = -EIO;
goto err_out_free_res;
}
+
/* If PHY status change register is still set to zero it means the
- * bootloader didn't initialize it */
+ * bootloader didn't initialize it, so we set it to:
+ * - enable phy status change
+ * - enable all phy addresses
+ * - set to lowest timer divider */
if (ioread16(ioaddr + PHY_CC) == 0)
- iowrite16(0x9f07, ioaddr + PHY_CC);
+ iowrite16(SCEN | PHY_MAX_ADDR << PHYAD_SHIFT |
+ 7 << TMRDIV_SHIFT, ioaddr + PHY_CC);
/* Init system & device */
lp->base = ioaddr;
--
1.7.5.4
^ permalink raw reply related
* [PATCH 2/8] r6040: remove unused variable mcr1 from private structure
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
drivers/net/ethernet/rdc/r6040.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index c10b0b8..fa29596 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -187,7 +187,7 @@ struct r6040_private {
dma_addr_t rx_ring_dma;
dma_addr_t tx_ring_dma;
u16 tx_free_desc;
- u16 mcr0, mcr1;
+ u16 mcr0;
struct net_device *dev;
struct mii_bus *mii_bus;
struct napi_struct napi;
--
1.7.5.4
^ permalink raw reply related
* [PATCH 5/8] r6040: define and use MLSR register bits
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>
Define the MLSR (MAC Last Status Register bits) for:
- tx fifo under-run
- tx exceed collision
- tx late collision
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
drivers/net/ethernet/rdc/r6040.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index db33573..1a365ae 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -77,6 +77,9 @@
#define MR_BSR 0x18 /* RX buffer size */
#define MR_DCR 0x1A /* RX descriptor control */
#define MLSR 0x1C /* Last status */
+#define TX_FIFO_UNDR 0x0200 /* TX FIFO under-run */
+#define TX_EXCEEDC 0x2000 /* Transmit exceed collision */
+#define TX_LATEC 0x4000 /* Transmit late collision */
#define MMDIO 0x20 /* MDIO control register */
#define MDIO_WRITE 0x4000 /* MDIO write */
#define MDIO_READ 0x2000 /* MDIO read */
@@ -604,9 +607,9 @@ static void r6040_tx(struct net_device *dev)
/* Check for errors */
err = ioread16(ioaddr + MLSR);
- if (err & 0x0200)
+ if (err & TX_FIFO_UNDR)
dev->stats.tx_fifo_errors++;
- if (err & (0x2000 | 0x4000))
+ if (err & (TX_EXCEEDC | TX_LATEC))
dev->stats.tx_carrier_errors++;
if (descptr->status & DSC_OWNER_MAC)
--
1.7.5.4
^ permalink raw reply related
* [PATCH 0/8] r6040: misc cleanups
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
Hi David,
This series contains some cleanups and one small fix, they are targetted
at net-next.
Thanks!
Florian Fainelli (8):
r6040: consolidate MAC reset to its own function
r6040: remove unused variable mcr1 from private structure
r6040: add a MAC operation timeout define
r6040: fix typo on stats update in tx path
r6040: define and use MLSR register bits
r6040: define and use MTPR transmit enable bit
r6040: define and use bits of register PHY_CC
r6040: update copyright from 2007 to now
drivers/net/ethernet/rdc/r6040.c | 75 ++++++++++++++++++++++----------------
1 files changed, 44 insertions(+), 31 deletions(-)
--
1.7.5.4
^ permalink raw reply
* [PATCH 1/8] r6040: consolidate MAC reset to its own function
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>
The reset of the MAC is currently done identically from two places
and one place is not waiting for the MAC_SM bit to be set after reset.
Everytime the MAC is software resetted a state machine is also needed
so consolidate the reset to its own function.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
drivers/net/ethernet/rdc/r6040.c | 37 ++++++++++++++++++-------------------
1 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index a26307f..c10b0b8 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -358,27 +358,35 @@ err_exit:
return rc;
}
-static void r6040_init_mac_regs(struct net_device *dev)
+static void r6040_reset_mac(struct r6040_private *lp)
{
- struct r6040_private *lp = netdev_priv(dev);
void __iomem *ioaddr = lp->base;
int limit = 2048;
u16 cmd;
- /* Mask Off Interrupt */
- iowrite16(MSK_INT, ioaddr + MIER);
-
- /* Reset RDC MAC */
iowrite16(MAC_RST, ioaddr + MCR1);
while (limit--) {
cmd = ioread16(ioaddr + MCR1);
if (cmd & MAC_RST)
break;
}
+
/* Reset internal state machine */
iowrite16(MAC_SM_RST, ioaddr + MAC_SM);
iowrite16(0, ioaddr + MAC_SM);
mdelay(5);
+}
+
+static void r6040_init_mac_regs(struct net_device *dev)
+{
+ struct r6040_private *lp = netdev_priv(dev);
+ void __iomem *ioaddr = lp->base;
+
+ /* Mask Off Interrupt */
+ iowrite16(MSK_INT, ioaddr + MIER);
+
+ /* Reset RDC MAC */
+ r6040_reset_mac(lp);
/* MAC Bus Control Register */
iowrite16(MBCR_DEFAULT, ioaddr + MBCR);
@@ -445,18 +453,13 @@ static void r6040_down(struct net_device *dev)
{
struct r6040_private *lp = netdev_priv(dev);
void __iomem *ioaddr = lp->base;
- int limit = 2048;
u16 *adrp;
- u16 cmd;
/* Stop MAC */
iowrite16(MSK_INT, ioaddr + MIER); /* Mask Off Interrupt */
- iowrite16(MAC_RST, ioaddr + MCR1); /* Reset RDC MAC */
- while (limit--) {
- cmd = ioread16(ioaddr + MCR1);
- if (cmd & MAC_RST)
- break;
- }
+
+ /* Reset RDC MAC */
+ r6040_reset_mac(lp);
/* Restore MAC Address to MIDx */
adrp = (u16 *) dev->dev_addr;
@@ -736,11 +739,7 @@ static void r6040_mac_address(struct net_device *dev)
u16 *adrp;
/* Reset MAC */
- iowrite16(MAC_RST, ioaddr + MCR1);
- /* Reset internal state machine */
- iowrite16(MAC_SM_RST, ioaddr + MAC_SM);
- iowrite16(0, ioaddr + MAC_SM);
- mdelay(5);
+ r6040_reset_mac(lp);
/* Restore MAC Address */
adrp = (u16 *) dev->dev_addr;
--
1.7.5.4
^ permalink raw reply related
* [PATCH 3/8] r6040: add a MAC operation timeout define
From: Florian Fainelli @ 2012-04-11 17:18 UTC (permalink / raw)
To: davem; +Cc: netdev, Florian Fainelli
In-Reply-To: <1334164723-9627-1-git-send-email-florian@openwrt.org>
2048 is the usual value for busy-waiting on a register r/w, define it
as MAC_DEF_TIMEOUT and use it where it is appropriate.
Signed-off-by: Florian Fainelli <florian@openwrt.org>
---
drivers/net/ethernet/rdc/r6040.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/rdc/r6040.c b/drivers/net/ethernet/rdc/r6040.c
index fa29596..9ffbf6e 100644
--- a/drivers/net/ethernet/rdc/r6040.c
+++ b/drivers/net/ethernet/rdc/r6040.c
@@ -137,6 +137,8 @@
#define MBCR_DEFAULT 0x012A /* MAC Bus Control Register */
#define MCAST_MAX 3 /* Max number multicast addresses to filter */
+#define MAC_DEF_TIMEOUT 2048 /* Default MAC read/write operation timeout */
+
/* Descriptor status */
#define DSC_OWNER_MAC 0x8000 /* MAC is the owner of this descriptor */
#define DSC_RX_OK 0x4000 /* RX was successful */
@@ -204,7 +206,7 @@ static char version[] __devinitdata = DRV_NAME
/* Read a word data from PHY Chip */
static int r6040_phy_read(void __iomem *ioaddr, int phy_addr, int reg)
{
- int limit = 2048;
+ int limit = MAC_DEF_TIMEOUT;
u16 cmd;
iowrite16(MDIO_READ + reg + (phy_addr << 8), ioaddr + MMDIO);
@@ -222,7 +224,7 @@ static int r6040_phy_read(void __iomem *ioaddr, int phy_addr, int reg)
static void r6040_phy_write(void __iomem *ioaddr,
int phy_addr, int reg, u16 val)
{
- int limit = 2048;
+ int limit = MAC_DEF_TIMEOUT;
u16 cmd;
iowrite16(val, ioaddr + MMWD);
@@ -361,7 +363,7 @@ err_exit:
static void r6040_reset_mac(struct r6040_private *lp)
{
void __iomem *ioaddr = lp->base;
- int limit = 2048;
+ int limit = MAC_DEF_TIMEOUT;
u16 cmd;
iowrite16(MAC_RST, ioaddr + MCR1);
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH 05/10] net: move destructor_arg to the front of sk_buff.
From: Ian Campbell @ 2012-04-11 17:00 UTC (permalink / raw)
To: Alexander Duyck
Cc: Eric Dumazet, netdev@vger.kernel.org, David Miller,
Michael S. Tsirkin, Wei Liu (Intern), xen-devel@lists.xen.org
In-Reply-To: <4F85B1EA.9000600@intel.com>
On Wed, 2012-04-11 at 17:31 +0100, Alexander Duyck wrote:
> On 04/11/2012 01:00 AM, Ian Campbell wrote:
> > On Tue, 2012-04-10 at 20:15 +0100, Alexander Duyck wrote:
> >> On 04/10/2012 11:41 AM, Eric Dumazet wrote:
> >>> On Tue, 2012-04-10 at 11:33 -0700, Alexander Duyck wrote:
> >>>
> >>>> Have you checked this for 32 bit as well as 64? Based on my math your
> >>>> next patch will still mess up the memset on 32 bit with the structure
> >>>> being split somewhere just in front of hwtstamps.
> >>>>
> >>>> Why not just take frags and move it to the start of the structure? It
> >>>> is already an unknown value because it can be either 16 or 17 depending
> >>>> on the value of PAGE_SIZE, and since you are making changes to frags the
> >>>> changes wouldn't impact the alignment of the other values later on since
> >>>> you are aligning the end of the structure. That way you would be
> >>>> guaranteed that all of the fields that will be memset would be in the
> >>>> last 64 bytes.
> >>>>
> >>> Now when a fragmented packet is copied in pskb_expand_head(), you access
> >>> two separate zones of memory to copy the shinfo. But its supposed to be
> >>> slow path.
> >>>
> >>> Problem with this is that the offsets of often used fields will be big
> >>> (instead of being < 127) and code will be bigger on x86.
> >> Actually now that I think about it my concerns go much further than the
> >> memset. I'm convinced that this is going to cause a pretty significant
> >> performance regression on multiple drivers, especially on non x86_64
> >> architecture. What we have right now on most platforms is a
> >> skb_shared_info structure in which everything up to and including frag 0
> >> is all in one cache line. This gives us pretty good performance for igb
> >> and ixgbe since that is our common case when jumbo frames are not
> >> enabled is to split the head and place the data in a page.
> > With all the changes in this series it is still possible to fit a
> > maximum standard MTU frame and the shinfo on the same 4K page while also
> > have the skb_shared_info up to and including frag [0] aligned to the
> > same 64 byte cache line.
> >
> > The only exception is destructor_arg on 64 bit which is on the preceding
> > cache line but that is not a field used in any hot path.
> The problem I have is that this is only true on x86_64. Proper work
> hasn't been done to guarantee this on any other architectures.
FWIW I did also explicitly cover i386 (see
<1334130984.12209.195.camel@dagon.hellion.org.uk>)
> I think what I would like to see is instead of just setting things up
> and hoping it comes out cache aligned on nr_frags why not take steps to
> guarantee it? You could do something like place and size the structure
> based on:
> SKB_DATA_ALIGN(sizeof(skb_shared_info) - offsetof(struct
> skb_shared_info, nr_frags)) + offsetof(struct skb_shared_info, nr_frags)
>
> That way you would have your alignment still guaranteed based off of the
> end of the structure, but anything placed before nr_frags would be
> placed on the end of the previous cache line.
>
> >> However the change being recommend here only resolves the issue for one
> >> specific architecture, and that is what I don't agree with. What we
> >> need is a solution that also works for 64K pages or 32 bit pointers and
> >> I am fairly certain this current solution does not.
> > I think it does work for 32 bit pointers. What issue to do you see with
> > 64K pages?
> >
> > Ian.
> With 64K pages the MAX_SKB_FRAGS value drops from 17 to 16. That will
> undoubtedly mess up the alignment.
Oh, I see. Need to think about this some more but your suggestion above
is an interesting one, I'll see what I can do with that.
Ian.
^ permalink raw reply
* Re: [RFC PATCH 1/2] net: ethtool: Add capability to retrieve plug-in module EEPROM
From: Stuart Hodgson @ 2012-04-11 16:50 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev, bruce.w.allan, mirq-linux, decot, amit.salecha,
alexander.h.duyck, davem, linux-kernel
In-Reply-To: <1333389160.2623.30.camel@bwh-desktop.uk.solarflarecom.com>
On 02/04/12 18:52, Ben Hutchings wrote:
> We previously discussed the need for this in person, so I'm going to
> review this as a submission rather than an RFC.
>
> On Tue, 2012-03-27 at 18:51 +0100, Stuart Hodgson wrote:
>> Added extensions to the ethtool API to obtain plugin module eeprom data
>> This is useful for end users to be able to determine what the capabilities
>> of the in use module are.
>>
>> The first provides a new struct ethtool_modinfo that will return the
>> type and size of plug-in module eeprom (such as SFP+) for parsing
>> by userland program.
>>
>> The second provides the API to get the raw eeprom information
>> using the existing ethtool_eeprom structture to return the data
>>
>> Signed-off-by: Stuart Hodgson<smhodgson@solarflare.com>
>> ---
>> include/linux/ethtool.h | 20 ++++++++++++
>> net/core/ethtool.c | 79
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 99 insertions(+), 0 deletions(-)
>
> This is line-wrapped; you'll need to take care to avoid that when
> submitting the patch for real. Tabs have been converted to spaces,
> which you also need to avoid. See Documentation/email-clients.txt.
>
>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>> index da5b2de..50eaf35 100644
>> --- a/include/linux/ethtool.h
>> +++ b/include/linux/ethtool.h
>> @@ -117,6 +117,14 @@ struct ethtool_eeprom {
>> __u8 data[0];
>> };
>>
>> +/* for passing plug-in module information */
>> +struct ethtool_modinfo {
>> + __u32 cmd;
>> + __u32 type;
>> + __u32 eeprom_len;
>> + __u32 reserved[8];
>> +};
>> +
>
> This needs a proper kernel-doc comment with a description of each of the
> fields (except reserved).
>
>> /**
>> * struct ethtool_coalesce - coalescing parameters for IRQs and stats
>> updates
>> * @cmd: ETHTOOL_{G,S}COALESCE
>> @@ -936,6 +944,10 @@ struct ethtool_ops {
>> int (*get_dump_data)(struct net_device *,
>> struct ethtool_dump *, void *);
>> int (*set_dump)(struct net_device *, struct ethtool_dump *);
>> + int (*get_module_info)(struct net_device *,
>> + struct ethtool_modinfo *);
>> + int (*get_module_eeprom)(struct net_device *,
>> + struct ethtool_eeprom *, u8 *);
>
> These need to be described in the kernel-doc comment for this structure.
>
>> };
>> #endif /* __KERNEL__ */
>> @@ -1010,6 +1022,8 @@ struct ethtool_ops {
>> #define ETHTOOL_SET_DUMP 0x0000003e /* Set dump settings */
>> #define ETHTOOL_GET_DUMP_FLAG 0x0000003f /* Get dump settings */
>> #define ETHTOOL_GET_DUMP_DATA 0x00000040 /* Get dump data */
>> +#define ETHTOOL_GMODULEINFO 0x00000041 /* Get plug-in module
>> information */
>> +#define ETHTOOL_GMODULEEEPROM 0x00000042 /* Get plug-in module eeprom */
>>
>> /* compatibility with older code */
>> #define SPARC_ETH_GSET ETHTOOL_GSET
>> @@ -1159,6 +1173,12 @@ struct ethtool_ops {
>> #define RX_CLS_LOC_FIRST 0xfffffffe
>> #define RX_CLS_LOC_LAST 0xfffffffd
>>
>> +/* EEPROM Standards for plug in modules */
>> +#define SFF_8079 0x1
>> +#define SFF_8079_LEN 256
>> +#define SFF_8472 0x2
>> +#define SFF_8472_LEN 512
>> +
>
> I think it's best to include a prefix of 'ethtool_' 'ETHTOOL_' or 'ETH_'
> in new definitions in ethtool.h. Since these are specific to modules I
> would suggest a prefix of 'ETH_MODULE_'.
>
>> /* Reset flags */
>> /* The reset() operation must clear the flags for the components which
>> * were actually reset. On successful return, the flags indicate the
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index 3f79db1..1e86bd9 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
> [...]
>> +static int ethtool_get_module_eeprom(struct net_device *dev,
>> + void __user *useraddr)
>> +{
>> + int ret;
>> + struct ethtool_eeprom eeprom;
>> + struct ethtool_modinfo modinfo;
>> + const struct ethtool_ops *ops = dev->ethtool_ops;
>> + void __user *userbuf = useraddr + sizeof(eeprom);
>> + u8 *data;
>> +
>> + if (!ops->get_module_info || !ops->get_module_eeprom)
>> + return -EOPNOTSUPP;
>> +
>> + if (copy_from_user(&eeprom, useraddr, sizeof(eeprom)))
>> + return -EFAULT;
>> +
>> + /* Check for wrap and zero */
>> + if (eeprom.offset + eeprom.len<= eeprom.offset)
>> + return -EINVAL;
>> +
>> + /* Get the modinfo to get the length */
>> + ret = ops->get_module_info(dev,&modinfo);
>> + if (ret)
>> + return ret;
>> +
>> + if (eeprom.offset + eeprom.len> modinfo.eeprom_len)
>> + return -EINVAL;
>> +
>> + data = kmalloc(PAGE_SIZE, GFP_USER);
>> + if (!data)
>> + return -ENOMEM;
>
> What if some device has a larger EEPROM? Surely this length should be
> eeprom.len.
>
Do you mean what if the eeprom length in te device is larger than
PAGE_SIZE? If so then it should really use modinfo.eeprom_len since
this the size of the data. eeprom.len could be arbitary.
>> + ret = ops->get_module_eeprom(dev,&eeprom, data);
>> + if (ret)
>> + goto out;
>> +
>> +
>> + if (copy_to_user(userbuf, data, eeprom.len)) {
>> + ret = -EFAULT;
>> + goto out;
>> + }
>> +
>> + if (copy_to_user(useraddr,&eeprom, sizeof(eeprom)))
>> + ret = -EFAULT;
> [...]
>
> I think you can drop this last copy as there's no information to return
> in the eeprom structure itself.
>
> Ben.
>
Other comments have been addressed and will be re-submitted.
Stu
^ permalink raw reply
* Re: [PATCH 05/10] net: move destructor_arg to the front of sk_buff.
From: Alexander Duyck @ 2012-04-11 16:31 UTC (permalink / raw)
To: Ian Campbell
Cc: Eric Dumazet, netdev@vger.kernel.org, David Miller,
Michael S. Tsirkin, Wei Liu (Intern), xen-devel@lists.xen.org
In-Reply-To: <1334131241.12209.199.camel@dagon.hellion.org.uk>
On 04/11/2012 01:00 AM, Ian Campbell wrote:
> On Tue, 2012-04-10 at 20:15 +0100, Alexander Duyck wrote:
>> On 04/10/2012 11:41 AM, Eric Dumazet wrote:
>>> On Tue, 2012-04-10 at 11:33 -0700, Alexander Duyck wrote:
>>>
>>>> Have you checked this for 32 bit as well as 64? Based on my math your
>>>> next patch will still mess up the memset on 32 bit with the structure
>>>> being split somewhere just in front of hwtstamps.
>>>>
>>>> Why not just take frags and move it to the start of the structure? It
>>>> is already an unknown value because it can be either 16 or 17 depending
>>>> on the value of PAGE_SIZE, and since you are making changes to frags the
>>>> changes wouldn't impact the alignment of the other values later on since
>>>> you are aligning the end of the structure. That way you would be
>>>> guaranteed that all of the fields that will be memset would be in the
>>>> last 64 bytes.
>>>>
>>> Now when a fragmented packet is copied in pskb_expand_head(), you access
>>> two separate zones of memory to copy the shinfo. But its supposed to be
>>> slow path.
>>>
>>> Problem with this is that the offsets of often used fields will be big
>>> (instead of being < 127) and code will be bigger on x86.
>> Actually now that I think about it my concerns go much further than the
>> memset. I'm convinced that this is going to cause a pretty significant
>> performance regression on multiple drivers, especially on non x86_64
>> architecture. What we have right now on most platforms is a
>> skb_shared_info structure in which everything up to and including frag 0
>> is all in one cache line. This gives us pretty good performance for igb
>> and ixgbe since that is our common case when jumbo frames are not
>> enabled is to split the head and place the data in a page.
> With all the changes in this series it is still possible to fit a
> maximum standard MTU frame and the shinfo on the same 4K page while also
> have the skb_shared_info up to and including frag [0] aligned to the
> same 64 byte cache line.
>
> The only exception is destructor_arg on 64 bit which is on the preceding
> cache line but that is not a field used in any hot path.
The problem I have is that this is only true on x86_64. Proper work
hasn't been done to guarantee this on any other architectures.
I think what I would like to see is instead of just setting things up
and hoping it comes out cache aligned on nr_frags why not take steps to
guarantee it? You could do something like place and size the structure
based on:
SKB_DATA_ALIGN(sizeof(skb_shared_info) - offsetof(struct
skb_shared_info, nr_frags)) + offsetof(struct skb_shared_info, nr_frags)
That way you would have your alignment still guaranteed based off of the
end of the structure, but anything placed before nr_frags would be
placed on the end of the previous cache line.
>> However the change being recommend here only resolves the issue for one
>> specific architecture, and that is what I don't agree with. What we
>> need is a solution that also works for 64K pages or 32 bit pointers and
>> I am fairly certain this current solution does not.
> I think it does work for 32 bit pointers. What issue to do you see with
> 64K pages?
>
> Ian.
With 64K pages the MAX_SKB_FRAGS value drops from 17 to 16. That will
undoubtedly mess up the alignment.
Thanks,
Alex
^ permalink raw reply
* [PATCH] atl1: fix kernel panic in case of DMA errors
From: Tony Zelenoff @ 2012-04-11 16:15 UTC (permalink / raw)
To: davem; +Cc: antonz, netdev
Problem:
There was two separate work_struct structures which share one
handler. Unfortunately getting atl1_adapter structure from
work_struct in case of DMA error was done from incorrect
offset which cause kernel panics.
Solution:
The useless work_struct for DMA error removed and
handler name changed to more generic one.
Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
drivers/net/ethernet/atheros/atlx/atl1.c | 12 +++++-------
drivers/net/ethernet/atheros/atlx/atl1.h | 3 +--
drivers/net/ethernet/atheros/atlx/atlx.c | 2 +-
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 40ac414..c926857 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -2476,7 +2476,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
"pcie phy link down %x\n", status);
if (netif_running(adapter->netdev)) { /* reset MAC */
iowrite32(0, adapter->hw.hw_addr + REG_IMR);
- schedule_work(&adapter->pcie_dma_to_rst_task);
+ schedule_work(&adapter->reset_dev_task);
return IRQ_HANDLED;
}
}
@@ -2488,7 +2488,7 @@ static irqreturn_t atl1_intr(int irq, void *data)
"pcie DMA r/w error (status = 0x%x)\n",
status);
iowrite32(0, adapter->hw.hw_addr + REG_IMR);
- schedule_work(&adapter->pcie_dma_to_rst_task);
+ schedule_work(&adapter->reset_dev_task);
return IRQ_HANDLED;
}
@@ -2633,10 +2633,10 @@ static void atl1_down(struct atl1_adapter *adapter)
atl1_clean_rx_ring(adapter);
}
-static void atl1_tx_timeout_task(struct work_struct *work)
+static void atl1_reset_dev_task(struct work_struct *work)
{
struct atl1_adapter *adapter =
- container_of(work, struct atl1_adapter, tx_timeout_task);
+ container_of(work, struct atl1_adapter, reset_dev_task);
struct net_device *netdev = adapter->netdev;
netif_device_detach(netdev);
@@ -3038,12 +3038,10 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
(unsigned long)adapter);
adapter->phy_timer_pending = false;
- INIT_WORK(&adapter->tx_timeout_task, atl1_tx_timeout_task);
+ INIT_WORK(&adapter->reset_dev_task, atl1_reset_dev_task);
INIT_WORK(&adapter->link_chg_task, atlx_link_chg_task);
- INIT_WORK(&adapter->pcie_dma_to_rst_task, atl1_tx_timeout_task);
-
err = register_netdev(netdev);
if (err)
goto err_common;
diff --git a/drivers/net/ethernet/atheros/atlx/atl1.h b/drivers/net/ethernet/atheros/atlx/atl1.h
index 109d6da..e04bf4d 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.h
+++ b/drivers/net/ethernet/atheros/atlx/atl1.h
@@ -758,9 +758,8 @@ struct atl1_adapter {
u16 link_speed;
u16 link_duplex;
spinlock_t lock;
- struct work_struct tx_timeout_task;
+ struct work_struct reset_dev_task;
struct work_struct link_chg_task;
- struct work_struct pcie_dma_to_rst_task;
struct timer_list phy_config_timer;
bool phy_timer_pending;
diff --git a/drivers/net/ethernet/atheros/atlx/atlx.c b/drivers/net/ethernet/atheros/atlx/atlx.c
index 3cd8837..c9e9dc5 100644
--- a/drivers/net/ethernet/atheros/atlx/atlx.c
+++ b/drivers/net/ethernet/atheros/atlx/atlx.c
@@ -194,7 +194,7 @@ static void atlx_tx_timeout(struct net_device *netdev)
{
struct atlx_adapter *adapter = netdev_priv(netdev);
/* Do the reset outside of interrupt context */
- schedule_work(&adapter->tx_timeout_task);
+ schedule_work(&adapter->reset_dev_task);
}
/*
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v2] Lockd: pass network namespace to creation and destruction routines
From: Stanislav Kinsbursky @ 2012-04-11 16:12 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org,
linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Pavel Emelianov, neilb-l3A5Bk7waGM@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
James Bottomley, davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
devel-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org
In-Reply-To: <20120411161114.GB28928-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
11.04.2012 20:11, J. Bruce Fields пишет:
> On Thu, Mar 29, 2012 at 06:54:33PM +0400, Stanislav Kinsbursky wrote:
>> v2: dereference of most probably already released nlm_host removed in
>> nlmclnt_done() and reclaimer().
>
> Did you want this in Trond's tree or mine?
>
Your tree is preferred since I'm working with it.
> --b.
>
>>
>> These routines are called from locks reclaimer() kernel thread. This thread
>> works in "init_net" network context and currently relays on persence on lockd
>> thread and it's per-net resources. Thus lockd_up() and lockd_down() can't relay
>> on current network context. So let's pass corrent one into them.
>>
>> Signed-off-by: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>
>> Signed-off-by: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>
>> ---
>> fs/lockd/clntlock.c | 13 ++++++++-----
>> fs/lockd/svc.c | 7 +++----
>> fs/nfsd/nfssvc.c | 6 +++---
>> include/linux/lockd/bind.h | 4 ++--
>> 4 files changed, 16 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
>> index ba1dc2e..ca0a080 100644
>> --- a/fs/lockd/clntlock.c
>> +++ b/fs/lockd/clntlock.c
>> @@ -56,7 +56,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
>> u32 nlm_version = (nlm_init->nfs_version == 2) ? 1 : 4;
>> int status;
>>
>> - status = lockd_up();
>> + status = lockd_up(nlm_init->net);
>> if (status< 0)
>> return ERR_PTR(status);
>>
>> @@ -65,7 +65,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
>> nlm_init->hostname, nlm_init->noresvport,
>> nlm_init->net);
>> if (host == NULL) {
>> - lockd_down();
>> + lockd_down(nlm_init->net);
>> return ERR_PTR(-ENOLCK);
>> }
>>
>> @@ -80,8 +80,10 @@ EXPORT_SYMBOL_GPL(nlmclnt_init);
>> */
>> void nlmclnt_done(struct nlm_host *host)
>> {
>> + struct net *net = host->net;
>> +
>> nlmclnt_release_host(host);
>> - lockd_down();
>> + lockd_down(net);
>> }
>> EXPORT_SYMBOL_GPL(nlmclnt_done);
>>
>> @@ -220,11 +222,12 @@ reclaimer(void *ptr)
>> struct nlm_wait *block;
>> struct file_lock *fl, *next;
>> u32 nsmstate;
>> + struct net *net = host->net;
>>
>> allow_signal(SIGKILL);
>>
>> down_write(&host->h_rwsem);
>> - lockd_up(); /* note: this cannot fail as lockd is already running */
>> + lockd_up(net); /* note: this cannot fail as lockd is already running */
>>
>> dprintk("lockd: reclaiming locks for host %s\n", host->h_name);
>>
>> @@ -275,6 +278,6 @@ restart:
>>
>> /* Release host handle after use */
>> nlmclnt_release_host(host);
>> - lockd_down();
>> + lockd_down(net);
>> return 0;
>> }
>> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
>> index b34100e..ce4c80e 100644
>> --- a/fs/lockd/svc.c
>> +++ b/fs/lockd/svc.c
>> @@ -295,11 +295,10 @@ static void lockd_down_net(struct net *net)
>> /*
>> * Bring up the lockd process if it's not already up.
>> */
>> -int lockd_up(void)
>> +int lockd_up(struct net *net)
>> {
>> struct svc_serv *serv;
>> int error = 0;
>> - struct net *net = current->nsproxy->net_ns;
>>
>> mutex_lock(&nlmsvc_mutex);
>> /*
>> @@ -377,12 +376,12 @@ EXPORT_SYMBOL_GPL(lockd_up);
>> * Decrement the user count and bring down lockd if we're the last.
>> */
>> void
>> -lockd_down(void)
>> +lockd_down(struct net *net)
>> {
>> mutex_lock(&nlmsvc_mutex);
>> if (nlmsvc_users) {
>> if (--nlmsvc_users) {
>> - lockd_down_net(current->nsproxy->net_ns);
>> + lockd_down_net(net);
>> goto out;
>> }
>> } else {
>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>> index fce472f..0f3e35b 100644
>> --- a/fs/nfsd/nfssvc.c
>> +++ b/fs/nfsd/nfssvc.c
>> @@ -220,7 +220,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
>> ret = nfsd_init_socks(port);
>> if (ret)
>> goto out_racache;
>> - ret = lockd_up();
>> + ret = lockd_up(&init_net);
>> if (ret)
>> goto out_racache;
>> ret = nfs4_state_start();
>> @@ -229,7 +229,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
>> nfsd_up = true;
>> return 0;
>> out_lockd:
>> - lockd_down();
>> + lockd_down(&init_net);
>> out_racache:
>> nfsd_racache_shutdown();
>> return ret;
>> @@ -246,7 +246,7 @@ static void nfsd_shutdown(void)
>> if (!nfsd_up)
>> return;
>> nfs4_state_shutdown();
>> - lockd_down();
>> + lockd_down(&init_net);
>> nfsd_racache_shutdown();
>> nfsd_up = false;
>> }
>> diff --git a/include/linux/lockd/bind.h b/include/linux/lockd/bind.h
>> index 11a966e..4d24d64 100644
>> --- a/include/linux/lockd/bind.h
>> +++ b/include/linux/lockd/bind.h
>> @@ -54,7 +54,7 @@ extern void nlmclnt_done(struct nlm_host *host);
>>
>> extern int nlmclnt_proc(struct nlm_host *host, int cmd,
>> struct file_lock *fl);
>> -extern int lockd_up(void);
>> -extern void lockd_down(void);
>> +extern int lockd_up(struct net *net);
>> +extern void lockd_down(struct net *net);
>>
>> #endif /* LINUX_LOCKD_BIND_H */
>>
--
Best regards,
Stanislav Kinsbursky
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] Lockd: pass network namespace to creation and destruction routines
From: J. Bruce Fields @ 2012-04-11 16:11 UTC (permalink / raw)
To: Stanislav Kinsbursky
Cc: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, xemul-bzQdu9zFT3WakBO8gow8eQ,
neilb-l3A5Bk7waGM, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jbottomley-bzQdu9zFT3WakBO8gow8eQ, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
devel-GEFAQzZX7r8dnm+yROfE0A
In-Reply-To: <20120329145421.31911.65512.stgit-bi+AKbBUZKagILUCTcTcHdKyNwTtLsGr@public.gmane.org>
On Thu, Mar 29, 2012 at 06:54:33PM +0400, Stanislav Kinsbursky wrote:
> v2: dereference of most probably already released nlm_host removed in
> nlmclnt_done() and reclaimer().
Did you want this in Trond's tree or mine?
--b.
>
> These routines are called from locks reclaimer() kernel thread. This thread
> works in "init_net" network context and currently relays on persence on lockd
> thread and it's per-net resources. Thus lockd_up() and lockd_down() can't relay
> on current network context. So let's pass corrent one into them.
>
> Signed-off-by: Stanislav Kinsbursky <skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>
> Signed-off-by: Stanislav Kinsbursky <skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>
> ---
> fs/lockd/clntlock.c | 13 ++++++++-----
> fs/lockd/svc.c | 7 +++----
> fs/nfsd/nfssvc.c | 6 +++---
> include/linux/lockd/bind.h | 4 ++--
> 4 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/fs/lockd/clntlock.c b/fs/lockd/clntlock.c
> index ba1dc2e..ca0a080 100644
> --- a/fs/lockd/clntlock.c
> +++ b/fs/lockd/clntlock.c
> @@ -56,7 +56,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
> u32 nlm_version = (nlm_init->nfs_version == 2) ? 1 : 4;
> int status;
>
> - status = lockd_up();
> + status = lockd_up(nlm_init->net);
> if (status < 0)
> return ERR_PTR(status);
>
> @@ -65,7 +65,7 @@ struct nlm_host *nlmclnt_init(const struct nlmclnt_initdata *nlm_init)
> nlm_init->hostname, nlm_init->noresvport,
> nlm_init->net);
> if (host == NULL) {
> - lockd_down();
> + lockd_down(nlm_init->net);
> return ERR_PTR(-ENOLCK);
> }
>
> @@ -80,8 +80,10 @@ EXPORT_SYMBOL_GPL(nlmclnt_init);
> */
> void nlmclnt_done(struct nlm_host *host)
> {
> + struct net *net = host->net;
> +
> nlmclnt_release_host(host);
> - lockd_down();
> + lockd_down(net);
> }
> EXPORT_SYMBOL_GPL(nlmclnt_done);
>
> @@ -220,11 +222,12 @@ reclaimer(void *ptr)
> struct nlm_wait *block;
> struct file_lock *fl, *next;
> u32 nsmstate;
> + struct net *net = host->net;
>
> allow_signal(SIGKILL);
>
> down_write(&host->h_rwsem);
> - lockd_up(); /* note: this cannot fail as lockd is already running */
> + lockd_up(net); /* note: this cannot fail as lockd is already running */
>
> dprintk("lockd: reclaiming locks for host %s\n", host->h_name);
>
> @@ -275,6 +278,6 @@ restart:
>
> /* Release host handle after use */
> nlmclnt_release_host(host);
> - lockd_down();
> + lockd_down(net);
> return 0;
> }
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index b34100e..ce4c80e 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -295,11 +295,10 @@ static void lockd_down_net(struct net *net)
> /*
> * Bring up the lockd process if it's not already up.
> */
> -int lockd_up(void)
> +int lockd_up(struct net *net)
> {
> struct svc_serv *serv;
> int error = 0;
> - struct net *net = current->nsproxy->net_ns;
>
> mutex_lock(&nlmsvc_mutex);
> /*
> @@ -377,12 +376,12 @@ EXPORT_SYMBOL_GPL(lockd_up);
> * Decrement the user count and bring down lockd if we're the last.
> */
> void
> -lockd_down(void)
> +lockd_down(struct net *net)
> {
> mutex_lock(&nlmsvc_mutex);
> if (nlmsvc_users) {
> if (--nlmsvc_users) {
> - lockd_down_net(current->nsproxy->net_ns);
> + lockd_down_net(net);
> goto out;
> }
> } else {
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index fce472f..0f3e35b 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -220,7 +220,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
> ret = nfsd_init_socks(port);
> if (ret)
> goto out_racache;
> - ret = lockd_up();
> + ret = lockd_up(&init_net);
> if (ret)
> goto out_racache;
> ret = nfs4_state_start();
> @@ -229,7 +229,7 @@ static int nfsd_startup(unsigned short port, int nrservs)
> nfsd_up = true;
> return 0;
> out_lockd:
> - lockd_down();
> + lockd_down(&init_net);
> out_racache:
> nfsd_racache_shutdown();
> return ret;
> @@ -246,7 +246,7 @@ static void nfsd_shutdown(void)
> if (!nfsd_up)
> return;
> nfs4_state_shutdown();
> - lockd_down();
> + lockd_down(&init_net);
> nfsd_racache_shutdown();
> nfsd_up = false;
> }
> diff --git a/include/linux/lockd/bind.h b/include/linux/lockd/bind.h
> index 11a966e..4d24d64 100644
> --- a/include/linux/lockd/bind.h
> +++ b/include/linux/lockd/bind.h
> @@ -54,7 +54,7 @@ extern void nlmclnt_done(struct nlm_host *host);
>
> extern int nlmclnt_proc(struct nlm_host *host, int cmd,
> struct file_lock *fl);
> -extern int lockd_up(void);
> -extern void lockd_down(void);
> +extern int lockd_up(struct net *net);
> +extern void lockd_down(struct net *net);
>
> #endif /* LINUX_LOCKD_BIND_H */
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [net-next PATCH v1 1/7] net: add generic PF_BRIDGE:RTM_ FDB hooks
From: Ben Hutchings @ 2012-04-11 16:05 UTC (permalink / raw)
To: John Fastabend
Cc: roprabhu, mst, stephen.hemminger, davem, hadi, jeffrey.t.kirsher,
netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <4F85991A.5030808@intel.com>
On Wed, 2012-04-11 at 07:45 -0700, John Fastabend wrote:
> On 4/10/2012 8:23 PM, Ben Hutchings wrote:
> > On Mon, 2012-04-09 at 15:00 -0700, John Fastabend wrote:
> > [...]
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index 1f77540..05822e5 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> > [...]
> >> @@ -905,6 +906,19 @@ struct netdev_fcoe_hbainfo {
> >> * feature set might be less than what was returned by ndo_fix_features()).
> >> * Must return >0 or -errno if it changed dev->features itself.
> >> *
> >> + * int (*ndo_fdb_add)(struct ndmsg *ndm, struct net_device *dev,
> >> + * unsigned char *addr, u16 flags)
> >> + * Adds an FDB entry to dev for addr. The ndmsg contains flags to indicate
> >> + * if the dev->master FDB should be updated or the devices internal FDB.
> >
> > I don't think the second sentence is helpful, as rtnl_fdb_add() will
> > take care of those flags.
> >
> >> + * int (*ndo_fdb_del)(struct ndmsg *ndm, struct net_device *dev,
> >> + * unsigned char *addr)
> >> + * Deletes the FDB entry from dev coresponding to addr. The ndmsg
> >> + * contains flags to indicate if the dev->master FDB should be
> >> + * updated or the devices internal FDB.
> >
> > Similarly here.
>
> agreed neither seem particularly helpful I'll remove them.
>
> >
> >> + * int (*ndo_fdb_dump)(struct sk_buff *skb, struct netlink_callback *cb,
> >> + * struct net_device *dev, int idx)
> >> + * Used to add FDB entries to dump requests. Implementers should add
> >> + * entries to skb and update idx with the number of entries.
> >> */
> > [...
> >> --- a/net/core/rtnetlink.c
> >> +++ b/net/core/rtnetlink.c
> > [...]
> >> +static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
> >> +{
> > [...]
> >> + err = -EOPNOTSUPP;
> >
> > So if NTF_MASTER and NTF_SELF are both set, we can quietly fall back to
> > just setting one FDB? Not sure that's really desirable though
> >
>
> It makes it easier to keep an embedded agent and the sw bridge in
> sync if setting both flags adds the entry to both the SW bridge and
> embedded bridge.
Yes, I agree with that.
[...]
> > Wonder what we should do on error here if we've already successfully
> > called ndo_fdb_add on the master? Should we try to roll back the first
> > addition?
> >
>
> The problem with rolling back is the table is likely already updated and
> traffic may already be being forwarded. So I think in this case the user
> will have to query the device to learn what failed. It seems like the
> simplest way to handle this. I think it is unwanted to have traffic being
> forwarded one way for a short period of time then rolled back.
>
> The other idea I just had is we could clear the NTF_ bit in ndm_flags after
> the successful add, del command. I believe the nlmsg gets sent back to the
> user on error I would need to check on this.
[...]
That sounds like a good way of doing it, assuming there's no
compatibility issue.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 05/10] net: move destructor_arg to the front of sk_buff.
From: Alexander Duyck @ 2012-04-11 16:05 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ian Campbell, netdev, David Miller, Michael S. Tsirkin, Wei Liu,
xen-devel
In-Reply-To: <1334132428.5300.2685.camel@edumazet-glaptop>
On 04/11/2012 01:20 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 12:15 -0700, Alexander Duyck wrote:
>
>> Actually now that I think about it my concerns go much further than the
>> memset. I'm convinced that this is going to cause a pretty significant
>> performance regression on multiple drivers, especially on non x86_64
>> architecture. What we have right now on most platforms is a
>> skb_shared_info structure in which everything up to and including frag 0
>> is all in one cache line. This gives us pretty good performance for igb
>> and ixgbe since that is our common case when jumbo frames are not
>> enabled is to split the head and place the data in a page.
> I dont understand this split thing for MTU=1500 frames.
>
> Even using half a page per fragment, each skb :
>
> needs 2 allocations for sk_buff and skb->head, plus one page alloc /
> reference.
>
> skb->truesize = ksize(skb->head) + sizeof(*skb) + PAGE_SIZE/2 = 512 +
> 256 + 2048 = 2816 bytes
The number you provide for head is currently only available for 128 byte
skb allocations. Anything larger than that will generate a 1K
allocation. Also after all of these patches the smallest size you can
allocate will be 1K for anything under 504 bytes.
The size advantage is actually more for smaller frames. In the case of
igb the behaviour is to place anything less than 512 bytes into just the
header and to skip using the page. As such we get a much more ideal
allocation for small packets. since the truesize is only 1280 in that case.
In the case of ixgbe the advantage is more of a cache miss advantage.
Ixgbe only receives the data into pages now. I can prefetch the first
two cache lines of the page into memory while allocating the skb to
receive it. As such we essentially cut the number of cache misses in
half versus the old approach which had us generating cache misses on the
sk_buff during allocation, and then generating more cache misses again
once we received the buffer and can fill out the sk_buff fields. A
similar size advantage exists as well, but only for frames 256 bytes or
smaller.
> With non split you have :
>
> 2 allocations for sk_buff and skb->head.
>
> skb->truesize = ksize(skb->head) + sizeof(*skb) = 2048 + 256 = 2304
> bytes
>
> less overhead and less calls to page allocator...
>
> This only can benefit if GRO is on, since aggregation can use fragments
> and a single sk_buff, instead of a frag_list
There is much more than true size involved here. My main argument is
that if we are going to align this modified skb_shared_info so that it
is aligned on nr_frags we should do it on all architectures, not just
x86_64.
Thanks,
Alex
^ permalink raw reply
* Who should find out the initial frequency of IBSS join?
From: Felipe Contreras @ 2012-04-11 15:53 UTC (permalink / raw)
To: netdev
Hi,
I'm a total noob at this, but I'm trying to properly fix an issue I'm
having while joining an ad-hoc network with wpa_supplicant; nl80211:
Join IBSS failed. Specifying any frequency makes it work.
Apparently I'm not the only one that has had this issue, however the
fix I found is in NetworkManager[1], but there's a lot of people that
don't use NetworkManager. I tried to file a bug in wpa_supplicant [2],
but they don't think it's a problem there.
Although I think I might be able to write a fix in wpa_supplicant, I
wonder if it might make sense to do it in the nl80211 driver. Wouldn't
it be possible for the driver to check if there's no frequency, and
just try any that is supported?
Cheers.
[1] https://gitorious.org/lanedo/networkmanager/commit/0b5ab39dbf14b4d3d34c4a37b10fa084d0fb272a/diffs
[2] http://w1.fi/bugz/show_bug.cgi?id=439
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v15 04/13] arch/x86: add syscall_get_arch to syscall.h
From: Will Drewry @ 2012-04-11 15:41 UTC (permalink / raw)
To: H. Peter Anvin
Cc: linux-kernel, linux-arch, linux-doc, kernel-hardening, netdev,
x86, arnd, davem, mingo, oleg, peterz, rdunlap, mcgrathr, tglx,
luto, eparis, serge.hallyn, djm, scarybeasts, indan, pmoore, akpm,
corbet, eric.dumazet, markus, coreyb, keescook
In-Reply-To: <4F84F895.4080101@zytor.com>
On Tue, Apr 10, 2012 at 10:20 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 04/10/2012 08:13 PM, Will Drewry wrote:
>> On Sun, Mar 25, 2012 at 2:34 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 03/14/2012 08:11 PM, Will Drewry wrote:
>>>>
>>>> +static inline int syscall_get_arch(struct task_struct *task,
>>>> + struct pt_regs *regs)
>>>> +{
>>>> +#ifdef CONFIG_IA32_EMULATION
>>>> + /*
>>>> + * TS_COMPAT is set for 32-bit syscall entries and then
>>>> + * remains set until we return to user mode.
>>>> + *
>>>> + * TIF_IA32 tasks should always have TS_COMPAT set at
>>>> + * system call time.
>>>> + */
>>>> + if (task_thread_info(task)->status & TS_COMPAT)
>>>> + return AUDIT_ARCH_I386;
>>>> +#endif
>>>> + return AUDIT_ARCH_X86_64;
>>>> +}
>>>> #endif /* CONFIG_X86_32 */
>>>>
>>>> #endif /* _ASM_X86_SYSCALL_H */
>>>
>>> Just one FYI on this: after the x32 changes are upstream this can be
>>> implemented in terms of is_ia32_task().
>>
>> Now that I've seen is_ia32_task(), it appears to be exactly the same as above:
>> (1) If we're x86_32, it's ia32
>> (2) If we're x86_64, ia32 == !!(status & TS_COMPAT)
>> (3) Otherwise, it's x86_64, including x32
>>
>> Am I missing something? Should is_ia32_task(void) take a task_struct?
>> Right now, I don't see any reason to change the code, as posted, but
>> maybe I am mis-reading?
>>
>
> Sorry, answered the wrong question. Yes, it is the same as above...
> just wandered if we could centralize this test. It might indeed make
> sense to provide general predicates which take a task pointer.
Makes sense to me. I'm leaving this specific patch alone at present.
That said, a quick grep shows only a handful of ia32 references:
./arch/x86/include/asm/compat.h: return is_ia32_task() || is_x32_task();
./arch/x86/ia32/ia32_signal.c: bool ia32 = is_ia32_task();
./arch/x86/kernel/ptrace.c: if (!is_ia32_task())
Would it make sense to make a new predicate or just expand the one
added in 3.4 to take a task_struct parameter? I'm not sure if there'd
be much fallout in converting these from directly checking
current_thread_info to task_thread_info.
It's a small patch either way.
cheers!
will
^ permalink raw reply
* Re: [RFC] net/bridge: port based vlan filtering for bridges
From: Stephen Hemminger @ 2012-04-11 15:38 UTC (permalink / raw)
To: Benjamin LaHaise; +Cc: netdev
In-Reply-To: <20120411153629.GC17739@kvack.org>
On Wed, 11 Apr 2012 11:36:29 -0400
Benjamin LaHaise <bcrl@kvack.org> wrote:
> On Wed, Apr 11, 2012 at 08:30:52AM -0700, Stephen Hemminger wrote:
> > On Wed, 11 Apr 2012 11:10:02 -0400
> > Benjamin LaHaise <bcrl@kvack.org> wrote:
> >
> > > Hello folks,
> > >
> > > Attached is the first stab at a patch to make it possible to filter packets
> > > received from other bridge ports based on the port number. This can be used
> > > to emulate port based VLANs that some switches support.
> > >
> > > The justification for this is a bit interesting. Initially, I had been
> > > filtering packets using firewall rules. Unfortunately, the number of
> > > filter rules becomes impossible to manage when trying to filter traffic
> > > between 100 different ports. CPU overhead of the filters is also a major
> > > problem.
> > >
> > > The particular use-case I'm dealing with is simulating wireless networks
> > > on a system using LXC containers. Each guest has a veth device that is a
> > > member of the bridge, but the topology of which nodes can "hear" each other
> > > changes at runtime.
> > >
> > > Comments/thoughts?
> > >
> >
> > Nak. If firewall doesn't work then implement a better netfilter
> > module.
>
> That still results in the CPU overhead of packet duplication for each and
> every bridge port regardless of the port receiving the packet or not.
> Hmmmm, would a NF_HOOK in should_deliver be okay?
Sure. Having better way to do policy would be great. Just don't want
to have implementations of specific policies in generic code.
^ permalink raw reply
* Re: [RFC] net/bridge: port based vlan filtering for bridges
From: Benjamin LaHaise @ 2012-04-11 15:36 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20120411083052.1ca6a6ef@nehalam.linuxnetplumber.net>
On Wed, Apr 11, 2012 at 08:30:52AM -0700, Stephen Hemminger wrote:
> On Wed, 11 Apr 2012 11:10:02 -0400
> Benjamin LaHaise <bcrl@kvack.org> wrote:
>
> > Hello folks,
> >
> > Attached is the first stab at a patch to make it possible to filter packets
> > received from other bridge ports based on the port number. This can be used
> > to emulate port based VLANs that some switches support.
> >
> > The justification for this is a bit interesting. Initially, I had been
> > filtering packets using firewall rules. Unfortunately, the number of
> > filter rules becomes impossible to manage when trying to filter traffic
> > between 100 different ports. CPU overhead of the filters is also a major
> > problem.
> >
> > The particular use-case I'm dealing with is simulating wireless networks
> > on a system using LXC containers. Each guest has a veth device that is a
> > member of the bridge, but the topology of which nodes can "hear" each other
> > changes at runtime.
> >
> > Comments/thoughts?
> >
>
> Nak. If firewall doesn't work then implement a better netfilter
> module.
That still results in the CPU overhead of packet duplication for each and
every bridge port regardless of the port receiving the packet or not.
Hmmmm, would a NF_HOOK in should_deliver be okay?
-ben
--
"Thought is the essence of where you are now."
^ permalink raw reply
* Re: [RFC] net/bridge: port based vlan filtering for bridges
From: Stephen Hemminger @ 2012-04-11 15:30 UTC (permalink / raw)
To: Benjamin LaHaise; +Cc: netdev
In-Reply-To: <20120411151002.GA17739@kvack.org>
On Wed, 11 Apr 2012 11:10:02 -0400
Benjamin LaHaise <bcrl@kvack.org> wrote:
> Hello folks,
>
> Attached is the first stab at a patch to make it possible to filter packets
> received from other bridge ports based on the port number. This can be used
> to emulate port based VLANs that some switches support.
>
> The justification for this is a bit interesting. Initially, I had been
> filtering packets using firewall rules. Unfortunately, the number of
> filter rules becomes impossible to manage when trying to filter traffic
> between 100 different ports. CPU overhead of the filters is also a major
> problem.
>
> The particular use-case I'm dealing with is simulating wireless networks
> on a system using LXC containers. Each guest has a veth device that is a
> member of the bridge, but the topology of which nodes can "hear" each other
> changes at runtime.
>
> Comments/thoughts?
>
Nak. If firewall doesn't work then implement a better netfilter
module.
^ permalink raw reply
* ipv6 multicast listener on linux box acting as multicast router
From: Massimiliano D'Angelo @ 2012-04-11 15:24 UTC (permalink / raw)
To: netdev
Hi guys,
I would like to have your opinion on a comment in the ip6mr.c file
(kernel 2.6.29.4). The comment follows:
/*
* RFC1584 teaches, that DVMRP/PIM router must deliver packets locally
* not only before forwarding, but after forwarding on all output
* interfaces. It is clear, if mrouter runs a multicasting
* program, it should receive packets not depending to what interface
* program is joined.
* If we will not make it, the program will have to join on all
* interfaces. On the other hand, multihoming host (or router, but
* not mrouter) cannot join to more than one interface - it will
* result in receiving multiple packets.
*/
Does the part "if we will not make it..." mean that such behaviour is
not yet implemented in the kernel? Looking at the code, it seems to me
that it is not yet implemented, but I would like someone to confirm
it.
As an implication, if I have a IPv6 multicast listener on a Linux box
acting as multicast router, do I need to have my application listening
on all the interfaces if I want to receive the multicast messages no
matter what is the interface from which they are received?
Thanks in advance for your help!
Massimiliano
^ 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