* Re: [PATCH v2 net-next 1/5] dsa: add support for Microchip KSZ tail tagging
From: Andrew Lunn @ 2017-05-13 13:03 UTC (permalink / raw)
To: Woojung.Huh
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A632AB@CHN-SV-EXMX02.mchp-main.com>
On Fri, May 12, 2017 at 08:07:54PM +0000, Woojung.Huh@microchip.com wrote:
> + padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;
> +
> + if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
> + nskb = skb;
> + } else {
> + nskb = alloc_skb(NET_IP_ALIGN + skb->len +
> + padlen + KSZ_INGRESS_TAG_LEN, GFP_ATOMIC);
Hi Woojung
Thanks for reusing the existing skb when possible. This should help
with performance.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 2/5] phy: micrel: add Microchip KSZ 9477 Switch PHY support
From: Andrew Lunn @ 2017-05-13 13:04 UTC (permalink / raw)
To: Woojung.Huh
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A632B7@CHN-SV-EXMX02.mchp-main.com>
On Fri, May 12, 2017 at 08:07:56PM +0000, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <Woojung.Huh@microchip.com>
>
> Adding Microchip 9477 Phy included in KSZ9477 Switch.
>
> Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 3/5] dsa: add DSA switch driver for Microchip KSZ9477
From: Andrew Lunn @ 2017-05-13 13:13 UTC (permalink / raw)
To: Woojung.Huh
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A632C2@CHN-SV-EXMX02.mchp-main.com>
On Fri, May 12, 2017 at 08:07:58PM +0000, Woojung.Huh@microchip.com wrote:
> +static void ksz_config_cpu_port(struct dsa_switch *ds)
> +{
> + struct ksz_device *dev = ds->priv;
> + int i;
> +
> + ds->num_ports = dev->port_cnt;
> +
> + for (i = 0; i < ds->num_ports; i++) {
> + if (dsa_is_cpu_port(ds, i)) {
> + dev->cpu_port = i;
> + /* enable tag tail for host port */
> + ksz_port_cfg(dev, i, REG_PORT_CTRL_0,
> + PORT_TAIL_TAG_ENABLE, true);
> + }
> + }
> +}
> +static const struct ksz_chip_data ksz_switch_chips[] = {
> + {
> + .chip_id = 0x00947700,
> + .dev_name = "KSZ9477",
> + .num_vlans = 4096,
> + .num_alus = 4096,
> + .num_statics = 16,
> + .enabled_ports = 0x1F, /* port0-4 */
> + .cpu_port = 5, /* port5 (RGMII) */
> + .port_cnt = 7,
> + .phy_port_cnt = 5,
> + },
> +};
Hi Woojung
Do we need cpu_port in this table? Can any port be used as a CPU port?
>From the code in ksz_config_cpu_port() it seems like it can.
And do we need enabled_ports? This seems to suggest only ports 0-4 can
be user ports?
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 3/5] dsa: add DSA switch driver for Microchip KSZ9477
From: Andrew Lunn @ 2017-05-13 14:56 UTC (permalink / raw)
To: Woojung.Huh
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A632C2@CHN-SV-EXMX02.mchp-main.com>
> +static int get_vlan_table(struct dsa_switch *ds, u16 vid, u32 *vlan_table)
> +{
> + struct ksz_device *dev = ds->priv;
> + u8 data;
> + int timeout = 1000;
> +
> + ksz_write16(dev, REG_SW_VLAN_ENTRY_INDEX__2, vid & VLAN_INDEX_M);
> + ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_READ | VLAN_START);
> + /* wait to be cleared */
> + data = 0;
> + do {
A blanks line before the comment would be nice.
> +static int ksz_reset_switch(struct dsa_switch *ds)
> +{
> + struct ksz_device *dev = ds->priv;
> + u8 data8;
> + u16 data16;
> + u32 data32;
> +
...
> +
> + memset(dev->vlan_cache, 0, sizeof(*dev->vlan_cache) * dev->num_vlans);
> +
> + return 0;
> +}
> +static int ksz_setup(struct dsa_switch *ds)
> +{
> + struct ksz_device *dev = ds->priv;
> + int ret = 0;
> +
> + dev->vlan_cache = devm_kmalloc_array(dev->dev,
> + sizeof(struct vlan_table),
> + dev->num_vlans, GFP_KERNEL);
You should check, but i think devm_kmalloc_array sets the allocated
memory to 0. So i don't think you need the memset. If it is needed, i
would move it here, after the check the allocation is successful.
> +static int ksz_enable_port(struct dsa_switch *ds, int port,
> + struct phy_device *phy)
> +{
> + struct ksz_device *dev = ds->priv;
> + u8 data8;
> + u16 data16;
> +
> + ksz_port_cfg(dev, port, REG_PORT_CTRL_0, PORT_MAC_LOOPBACK, false);
> +
> + /* set back pressure */
> + ksz_port_cfg(dev, port, REG_PORT_MAC_CTRL_1, PORT_BACK_PRESSURE, true);
> +
> + /* set flow control */
> + ksz_port_cfg(dev, port, REG_PORT_CTRL_0,
> + PORT_FORCE_TX_FLOW_CTRL | PORT_FORCE_RX_FLOW_CTRL, true);
> +
> + /* enable boradcast storm limit */
broadcast.
> +static int ksz_port_vlan_dump(struct dsa_switch *ds, int port,
> + struct switchdev_obj_port_vlan *vlan,
> + int (*cb)(struct switchdev_obj *obj))
> +{
> + struct ksz_device *dev = ds->priv;
> + u16 vid;
> + u16 data;
> + struct vlan_table *vlan_cache;
> + int err = 0;
> +
> + /* use dev->vlan_cache due to lack of searching valid vlan entry */
> + for (vid = vlan->vid_begin; vid < dev->num_vlans; vid++) {
> + vlan_cache = &dev->vlan_cache[vid];
> +
I wounder if the vlan cache should be protected my a mutex?
> + /* wait to be finished */
> + do {
> + ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
> + } while (data & ALU_START);
Missing timeouts if the hardware stops responding. There are a few
other similar loop i spotted. Please add a timeout for them all. Maybe
add a generic helper function if it makes sense?
> +static void ksz_port_mdb_add(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_mdb *mdb,
> + struct switchdev_trans *trans)
> +{
> + struct ksz_device *dev = ds->priv;
> + u32 static_table[4];
> + u32 data;
> + int index;
> + u32 mac_hi, mac_lo;
> +
> + mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
> + mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
> + mac_lo |= ((mdb->addr[4] << 8) | mdb->addr[5]);
> +
> + for (index = 0; index < dev->num_statics; index++) {
> + mutex_lock(&dev->alu_mutex);
> +
> + /* find empty slot first */
> + data = (index << ALU_STAT_INDEX_S) |
> + ALU_STAT_READ | ALU_STAT_START;
> + ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
> +
> + /* wait to be finished */
> + do {
> + ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
> + } while (data & ALU_STAT_START);
> +
> + /* read ALU static table */
> + read_table(ds, static_table);
> +
> + mutex_unlock(&dev->alu_mutex);
> +
> + if (static_table[0] & ALU_V_STATIC_VALID) {
> + /* check this has same vid & mac address */
> +
> + if (((static_table[2] >> ALU_V_FID_S) == (mdb->vid)) &&
> + ((static_table[2] & ALU_V_MAC_ADDR_HI) == mac_hi) &&
> + (static_table[3] == mac_lo)) {
> + /* found matching one */
> + break;
> + }
> + } else {
> + /* found empty one */
> + break;
Since the mutux has been released, could this empty entry be stolen by
some other parallel running thread? It seems like you should keep hold
of the mutex until you have at least marked it as used.
> + }
> + }
> +
> + /* no available entry */
> + if (index == dev->num_statics)
> + return;
> +
> + /* add entry */
> + static_table[0] = ALU_V_STATIC_VALID;
> + static_table[1] |= BIT(port);
> + if (mdb->vid)
> + static_table[1] |= ALU_V_USE_FID;
> + static_table[2] = (mdb->vid << ALU_V_FID_S);
> + static_table[2] |= mac_hi;
> + static_table[3] = mac_lo;
> +
> + mutex_lock(&dev->alu_mutex);
> +
> + write_table(ds, static_table);
> +
> + data = (index << ALU_STAT_INDEX_S) | ALU_STAT_START;
> + ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
> +
> + /* wait to be finished */
> + do {
> + ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
> + } while (data & ALU_STAT_START);
> +
> + mutex_unlock(&dev->alu_mutex);
> +}
> +
> +static int ksz_port_mdb_del(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_mdb *mdb)
> +{
> + struct ksz_device *dev = ds->priv;
> + u32 static_table[4];
> + u32 data;
> + int index;
> + u32 mac_hi, mac_lo;
> +
> + mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
> + mac_lo = ((mdb->addr[2] << 24) | (mdb->addr[3] << 16));
> + mac_lo |= ((mdb->addr[4] << 8) | mdb->addr[5]);
> +
> + for (index = 0; index < dev->num_statics; index++) {
> + mutex_lock(&dev->alu_mutex);
> +
> + /* find empty slot first */
> + data = (index << ALU_STAT_INDEX_S) |
> + ALU_STAT_READ | ALU_STAT_START;
> + ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
> +
> + /* wait to be finished */
> + do {
> + ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
> + } while (data & ALU_STAT_START);
> +
> + /* read ALU static table */
> + read_table(ds, static_table);
> +
> + mutex_unlock(&dev->alu_mutex);
> +
Here also, i wounder if it is safe the release the mutex and then
later claim it?
> + if (static_table[0] & ALU_V_STATIC_VALID) {
> + /* check this has same vid & mac address */
> +
> + if (((static_table[2] >> ALU_V_FID_S) == (mdb->vid)) &&
> + ((static_table[2] & ALU_V_MAC_ADDR_HI) == mac_hi) &&
> + (static_table[3] == mac_lo)) {
> + /* found matching one */
> + break;
> + }
> + }
> + }
> +
> + /* no available entry */
> + if (index == dev->num_statics)
> + return -EINVAL;
> +
> + /* clear port */
> + static_table[1] &= ~BIT(port);
> +
> + if ((static_table[1] & ALU_V_PORT_MAP) == 0) {
> + /* delete entry */
> + static_table[0] = 0;
> + static_table[1] = 0;
> + static_table[2] = 0;
> + static_table[3] = 0;
> + }
> +
> + mutex_lock(&dev->alu_mutex);
> +
> + write_table(ds, static_table);
> +
> + data = (index << ALU_STAT_INDEX_S) | ALU_STAT_START;
> + ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
> +
> + /* wait to be finished */
> + do {
> + ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
> + } while (data & ALU_STAT_START);
> +
> + mutex_unlock(&dev->alu_mutex);
> +
> + return 0;
> +}
> +int ksz_switch_detect(struct ksz_device *dev)
> +{
> + u8 data8;
> + u32 id32;
> + int ret;
> +
> + /* turn off SPI DO Edge select */
> + ret = ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
> + if (ret)
> + return ret;
> +
> + data8 &= ~SPI_AUTO_EDGE_DETECTION;
> + ret = ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
> + if (ret)
> + return ret;
> +
> + /* read chip id */
> + ret = ksz_read32(dev, REG_CHIP_ID0__1, &id32);
> + if (ret)
> + return ret;
> +
> + dev->chip_id = id32;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(ksz_switch_detect);
> +
> +int ksz_switch_register(struct ksz_device *dev)
> +{
> + int ret;
> +
> + if (dev->pdata) {
> + dev->chip_id = dev->pdata->chip_id;
> + dev->enabled_ports = dev->pdata->enabled_ports;
> + }
> +
> + if (!dev->chip_id && ksz_switch_detect(dev))
> + return -EINVAL;
Is there a need for this? Is there silicon with the wrong ID in it?
> +static inline int ksz_spi_read_reg(struct spi_device *spi, u32 reg, u8 *val,
> + unsigned int len)
Please leave the compiler to decide if to inline the function. The
same applies everywhere in .c files.
Overall the code is nice, just a few small issues to sort out.
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 4/5] dsa: Add spi support to Microchip KSZ switches
From: Andrew Lunn @ 2017-05-13 14:56 UTC (permalink / raw)
To: Woojung.Huh
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A632CA@CHN-SV-EXMX02.mchp-main.com>
On Fri, May 12, 2017 at 08:08:00PM +0000, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <Woojung.Huh@microchip.com>
>
> A sample SPI configuration for Microchip KSZ switches.
>
> Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 5/5] dsa: add maintainer of Microchip KSZ switches
From: Andrew Lunn @ 2017-05-13 14:58 UTC (permalink / raw)
To: Woojung.Huh
Cc: f.fainelli, vivien.didelot, sergei.shtylyov, netdev, davem,
UNGLinuxDriver
In-Reply-To: <9235D6609DB808459E95D78E17F2E43D40A632D5@CHN-SV-EXMX02.mchp-main.com>
On Fri, May 12, 2017 at 08:08:01PM +0000, Woojung.Huh@microchip.com wrote:
> From: Woojung Huh <Woojung.Huh@microchip.com>
>
> Adding maintainer of Microchip KSZ switches.
>
> Signed-off-by: Woojung Huh <Woojung.Huh@microchip.com>
> ---
> MAINTAINERS | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 45b173a..e65e501 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8370,6 +8370,15 @@ F: drivers/media/platform/atmel/atmel-isc.c
> F: drivers/media/platform/atmel/atmel-isc-regs.h
> F: devicetree/bindings/media/atmel-isc.txt
>
> +MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
> +M: Woojung Huh <Woojung.Huh@microchip.com>
> +M: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> +L: netdev@vger.kernel.org
> +S: Maintained
> +F: drivers/net/dsa/microchip/*
> +F: include/linux/platform_data/microchip-ksz.h
Please add the tag_* files to this list.
Otherwise:
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH] net: ipv6: Truncate single route when it doesn't fit into dump buffer.
From: David Ahern @ 2017-05-13 17:13 UTC (permalink / raw)
To: Jan Moskyto Matejka; +Cc: David Miller, mq, netdev, roopa
In-Reply-To: <20170513105401.iaialveoomahrmyq@lopatka.joja.cz>
On 5/13/17 4:54 AM, Jan Moskyto Matejka wrote:
>> I see 2 problems:
>> 1. the kernel is not telling the user the supplied buffer is too small
>> (ie., if a single route does not fit in the skb then it should fail and
>> return an error code to the user),
>
> Definitely. I want just to note that this condition usually occurs
> somewhere during route dump. To know it before starting output, we would
> have to walk the FIB once before dump to calculate max route len.
When adding a route to the skb, track whether it contains at least 1
route. If not, it means the next route in the dump is larger than the
given buffer. Detect this condition and error out of the dump -
returning an error to the user (-ENOSPC? or EMSGSIZE?)
>
>> 2. multipath routes for IPv4 and IPv6 do not have a limit.
>>
>> Should the kernel put a limit on the number of nexthops? I recently put
>> a cap on MPLS route size as 4096 bytes, but I think this should be
>> revisited in terms of a limit on number of nexthops to create a
>> consistent limit even if struct sizes change. And, the limit on the
>> number of nexthops should be consistent across address families (same
>> limit for IPv4, IPv6, and MPLS).
>>
>> From discussions I have had, 32 nexthops for a single route is on the
>> laughably high side, but some people do crazy things. How about a limit
>> of 256 nexthops?
>
> 256 should be OK even for a crazy developer of BIRD.
>
> It would be nice to have if the returned error were somehow useful for
> the userspace -- to know what is happening, not only something like
> "impossible to add / append route".
Top of tree kernel has extended error reporting so a message can be
returned that says something to the effect of "route size is larger than
supplied buffer size".
^ permalink raw reply
* Re: [PATCH] net: ipv6: Truncate single route when it doesn't fit into dump buffer.
From: Jan Moskyto Matejka @ 2017-05-13 17:29 UTC (permalink / raw)
To: David Ahern; +Cc: David Miller, mq, netdev, roopa
In-Reply-To: <f72f0622-f5d3-6a85-e8e8-15980c9f1f2d@gmail.com>
On Sat, May 13, 2017 at 11:13:38AM -0600, David Ahern wrote:
> On 5/13/17 4:54 AM, Jan Moskyto Matejka wrote:
> >> I see 2 problems:
> >> 1. the kernel is not telling the user the supplied buffer is too small
> >> (ie., if a single route does not fit in the skb then it should fail and
> >> return an error code to the user),
> >
> > Definitely. I want just to note that this condition usually occurs
> > somewhere during route dump. To know it before starting output, we would
> > have to walk the FIB once before dump to calculate max route len.
>
> When adding a route to the skb, track whether it contains at least 1
> route. If not, it means the next route in the dump is larger than the
> given buffer. Detect this condition and error out of the dump -
> returning an error to the user (-ENOSPC? or EMSGSIZE?)
EMSGSIZE seems OK for me.
^ permalink raw reply
* Re: arch: arm: bpf: Converting cBPF to eBPF for arm 32 bit
From: Shubham Bansal @ 2017-05-13 21:38 UTC (permalink / raw)
To: Kees Cook
Cc: David Miller, Mircea Gherzan, Network Development,
kernel-hardening@lists.openwall.com,
linux-arm-kernel@lists.infradead.org, ast, Daniel Borkmann
In-Reply-To: <CAGXu5j+2q4Wwfhuk_DnDaV894A9ZOD=A_AvAU_OZG6KQv0_KMg@mail.gmail.com>
Finally finished testing.
"test_bpf: Summary: 314 PASSED, 0 FAILED, [274/306 JIT'ed]"
Will send the patch after code refactoring. Thanks for all the help
you guys. I really really appreciate it.
Special thanks to Kees and Daniel. :)
Best,
Shubham Bansal
On Thu, May 11, 2017 at 9:00 PM, Kees Cook <keescook@chromium.org> wrote:
> On Thu, May 11, 2017 at 2:32 AM, Shubham Bansal
> <illusionist.neo@gmail.com> wrote:
>> What do you guys suggest i should implement it? I am almost done with
>> my current implementation but if you think I should change it to the
>> way David suggested, its better to suggest now before I send the
>> patch.
>
> I'd say send what you have right now, as it's a good starting point
> for future work. I'll be curious to see the benchmarks, etc. It can be
> a base for further optimization.
>
> Thanks for chipping away at this!
>
> -Kees
>
> --
> Kees Cook
> Pixel Security
^ permalink raw reply
* [PATCH] rtlwifi: rtl8723ae:fix spelling mistake: "Coexistance" -> "Coexistence"
From: Colin King @ 2017-05-13 22:37 UTC (permalink / raw)
To: Larry Finger, Chaoming Li, Kalle Valo, Joe Perches,
Christian Engelmayer, Arnd Bergmann, linux-wireless, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Trivial fix to spelling mistake in RT_TRACE text
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
index 859c045bd37c..47e5f9003bb0 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c
@@ -2313,7 +2313,7 @@ static void rtl8723e_bt_var_init(struct ieee80211_hw *hw)
rtlpriv->btcoexist.eeprom_bt_radio_shared;
RT_TRACE(rtlpriv, COMP_BT_COEXIST, DBG_TRACE,
- "BT Coexistance = 0x%x\n",
+ "BT Coexistence = 0x%x\n",
rtlpriv->btcoexist.bt_coexistence);
if (rtlpriv->btcoexist.bt_coexistence) {
--
2.11.0
^ permalink raw reply related
* 16589 netdev
From: kirola @ 2017-05-13 22:50 UTC (permalink / raw)
To: netdev
[-- Attachment #1: 16506.zip --]
[-- Type: application/zip, Size: 4874 bytes --]
^ permalink raw reply
* [PATCH iproute2] ip: add support for more MPLS labels
From: David Ahern @ 2017-05-14 1:27 UTC (permalink / raw)
To: netdev, stephen; +Cc: David Ahern
Kernel now supports up to 30 labels but not defined as part of the uapi.
iproute2 handles up to 8 labels but in a non-consistent way. Update ip
to handle more labels, but in a more programmatic way.
For the MPLS address family, the data field in inet_prefix is used for
labels. Increase that field to 64 u32's -- 64 as nothing more than a
convenient power of 2 number.
Update mpls_pton to take the length of the address field, convert that
length to number of labels and add better error handling to the parsing
of the user supplied string.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/utils.h | 7 ++-----
lib/mpls_pton.c | 11 +++++++----
lib/utils.c | 7 +++++--
3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/include/utils.h b/include/utils.h
index 8c12e1e2a60c..bfbc9e6dff55 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -61,7 +61,7 @@ typedef struct
__s16 bitlen;
/* These next two fields match rtvia */
__u16 family;
- __u32 data[8];
+ __u32 data[64];
} inet_prefix;
#define PREFIXLEN_SPECIFIED 1
@@ -88,9 +88,6 @@ struct ipx_addr {
# define AF_MPLS 28
#endif
-/* Maximum number of labels the mpls helpers support */
-#define MPLS_MAX_LABELS 8
-
__u32 get_addr32(const char *name);
int get_addr_1(inet_prefix *dst, const char *arg, int family);
int get_prefix_1(inet_prefix *dst, char *arg, int family);
@@ -155,7 +152,7 @@ const char *ipx_ntop(int af, const void *addr, char *str, size_t len);
int ipx_pton(int af, const char *src, void *addr);
const char *mpls_ntop(int af, const void *addr, char *str, size_t len);
-int mpls_pton(int af, const char *src, void *addr);
+int mpls_pton(int af, const char *src, void *addr, size_t alen);
extern int __iproute2_hz_internal;
int __get_hz(void);
diff --git a/lib/mpls_pton.c b/lib/mpls_pton.c
index bd448cfcf14a..6d2e6a69436a 100644
--- a/lib/mpls_pton.c
+++ b/lib/mpls_pton.c
@@ -7,12 +7,13 @@
#include "utils.h"
-static int mpls_pton1(const char *name, struct mpls_label *addr)
+static int mpls_pton1(const char *name, struct mpls_label *addr,
+ unsigned int maxlabels)
{
char *endp;
unsigned count;
- for (count = 0; count < MPLS_MAX_LABELS; count++) {
+ for (count = 0; count < maxlabels; count++) {
unsigned long label;
label = strtoul(name, &endp, 0);
@@ -37,17 +38,19 @@ static int mpls_pton1(const char *name, struct mpls_label *addr)
addr += 1;
}
/* The address was too long */
+ fprintf(stderr, "Error: too many labels.\n");
return 0;
}
-int mpls_pton(int af, const char *src, void *addr)
+int mpls_pton(int af, const char *src, void *addr, size_t alen)
{
+ unsigned int maxlabels = alen / sizeof(struct mpls_label);
int err;
switch(af) {
case AF_MPLS:
errno = 0;
- err = mpls_pton1(src, (struct mpls_label *)addr);
+ err = mpls_pton1(src, (struct mpls_label *)addr, maxlabels);
break;
default:
errno = EAFNOSUPPORT;
diff --git a/lib/utils.c b/lib/utils.c
index 6d5642f4f1f3..e77bd302530b 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -518,15 +518,18 @@ int get_addr_1(inet_prefix *addr, const char *name, int family)
}
if (family == AF_MPLS) {
+ unsigned int maxlabels;
int i;
addr->family = AF_MPLS;
- if (mpls_pton(AF_MPLS, name, addr->data) <= 0)
+ if (mpls_pton(AF_MPLS, name, addr->data,
+ sizeof(addr->data)) <= 0)
return -1;
addr->bytelen = 4;
addr->bitlen = 20;
/* How many bytes do I need? */
- for (i = 0; i < 8; i++) {
+ maxlabels = sizeof(addr->data) / sizeof(struct mpls_label);
+ for (i = 0; i < maxlabels; i++) {
if (ntohl(addr->data[i]) & MPLS_LS_S_MASK) {
addr->bytelen = (i + 1)*4;
break;
--
2.11.0 (Apple Git-81)
^ permalink raw reply related
* Re: [RFC] iproute: Add support for extended ack to rtnl_talk
From: David Ahern @ 2017-05-14 1:29 UTC (permalink / raw)
To: Phil Sutter, Stephen Hemminger, David Miller, daniel, netdev
In-Reply-To: <20170504204318.GB21130@orbyte.nwl.cc>
On 5/4/17 2:43 PM, Phil Sutter wrote:
> So in summary, given that very little change happens to iproute2's
> internal libnetlink, I don't see much urge to make it use libmnl as
> backend. In my opinion it just adds another potential source of errors.
>
> Eventually this should be a maintainer level decision, though. :)
What is the decision on this?
^ permalink raw reply
* Re: [PATCH] net/smc: mark as BROKEN due to remote memory exposure
From: Christoph Hellwig @ 2017-05-14 5:58 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: Bart Van Assche, davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
In-Reply-To: <1494514662.2489.1.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Dave,
this patch has not been superceeded by anything, can you explain why it
has been marked as such in patchworks?
On Thu, May 11, 2017 at 02:57:43PM +0000, Bart Van Assche wrote:
> On Wed, 2017-05-10 at 09:26 +0200, Christoph Hellwig wrote:
> > The driver has a lot of quality issues due to the lack of RDMA-side
> > review, and explicitly bypasses APIs to register all memory once a
> > connection is made, and thus allows remote access to memoery.
> >
> > Mark it as broken until at least that part is fixed.
>
> Since this is the only way to get the BROKEN marker in the v4.11 stable
> kernel series:
>
> Acked-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 net 1/1] smc: switch to usage of IB_PD_UNSAFE_GLOBAL_RKEY
From: Christoph Hellwig @ 2017-05-14 6:01 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Ursula Braun, davem-fT/PcQaiUtIeIZ0/mPfg9Q, hch-jcswGhMUV9g,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA,
jwi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
schwidefsky-tA70FqPdS9bQT0dZR+AlfA,
heiko.carstens-tA70FqPdS9bQT0dZR+AlfA,
raspl-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
In-Reply-To: <20170513112948.GK3616-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
On Sat, May 13, 2017 at 02:29:48PM +0300, Leon Romanovsky wrote:
> I'm glad that you moved to use IB_PD_UNSAFE_GLOBAL_RKEY, so the users
> will see the warning in their dmesg log.
>
> However can you please update your commit log? There is need to add
> description of security issue (access to whole physical memory), clear
> message that doesn't fix anything and remove mentioning unpredictable
> future (... as soon as possible ...).
Yes, please.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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
* [PATCH iproute2 master 0/4] pedit: Introduce IPv6 support + some minor fixes
From: Amir Vadai @ 2017-05-14 8:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Or Gerlitz, Amir Vadai
Hi,
This patchset introduces pedit IPv6 support.
Almost all IPv6 header fields are editable now (src, dst, flow_lbl,
payload_len, next_hdr and hoplimit).
The patch uses the new extended pedit netlink and will fail the operation if
kernel has no support or user didn't use the 'ex' keyword.
In addition to this patch, 3 more patches fix some minor UI issues:
- some typo's
- 'retain' can't be used with fields > 32 bits. It will make unexpected things
when used in such fields. Fixing this limitiation requires some changes (in
tc user space only) that are out of the scope of this patchset. So I added a
patch to prevent the user from using retain on those fields.
Thanks,
Amir
Amir Vadai (4):
pedit: Fix a typo in warning
pedit: Do not allow using retain for too big fields
pedit: Check for extended capability in protocol parser
pedit: Introduce ipv6 support
man/man8/tc-pedit.8 | 33 ++++++++++++++++++-
tc/Makefile | 1 +
tc/m_pedit.c | 51 ++++++++++++++++++++++++++++--
tc/p_eth.c | 3 ++
tc/p_ip.c | 17 +---------
tc/p_ip6.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++
tc/p_udp.c | 3 ++
7 files changed, 179 insertions(+), 20 deletions(-)
create mode 100644 tc/p_ip6.c
--
2.12.2
^ permalink raw reply
* [PATCH iproute2 master 1/4] pedit: Fix a typo in warning
From: Amir Vadai @ 2017-05-14 8:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Or Gerlitz, Amir Vadai
In-Reply-To: <20170514081746.9010-1-amir@vadai.me>
'ex' attribute should be placed after 'action pedit' and not after
'munge'.
Signed-off-by: Amir Vadai <amir@vadai.me>
---
tc/m_pedit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 6498dd91b471..7ef2acc52bce 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -146,7 +146,7 @@ int pack_key(struct m_pedit_sel *_sel, struct m_pedit_key *tkey)
if (tkey->htype != TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK ||
tkey->cmd != TCA_PEDIT_KEY_EX_CMD_SET) {
fprintf(stderr,
- "Munge parameters not supported. Use 'munge ex'.\n");
+ "Munge parameters not supported. Use 'pedit ex munge ...'.\n");
return -1;
}
}
--
2.12.2
^ permalink raw reply related
* [PATCH iproute2 master 2/4] pedit: Do not allow using retain for too big fields
From: Amir Vadai @ 2017-05-14 8:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Or Gerlitz, Amir Vadai
In-Reply-To: <20170514081746.9010-1-amir@vadai.me>
Using retain for fields longer than 32 bits is not supported.
Do not allow user to do it.
Signed-off-by: Amir Vadai <amir@vadai.me>
---
man/man8/tc-pedit.8 | 3 ++-
tc/m_pedit.c | 6 ++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8
index 7f482eafc6c7..9c4d57b972cc 100644
--- a/man/man8/tc-pedit.8
+++ b/man/man8/tc-pedit.8
@@ -266,7 +266,8 @@ Keep the addressed data as is.
.BI retain " RVAL"
This optional extra part of
.I CMD_SPEC
-allows to exclude bits from being changed.
+allows to exclude bits from being changed. Supported only for 32 bits fields
+or smaller.
.TP
.I CONTROL
The following keywords allow to control how the tree of qdisc, classes,
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 7ef2acc52bce..9b74c965932e 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -353,6 +353,12 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
argv++;
}
+ if (len > 4 && retain != ~0) {
+ fprintf(stderr,
+ "retain is not supported for fields longer the 32 bits\n");
+ return -1;
+ }
+
if (type == TMAC) {
res = pack_mac(sel, tkey, (__u8 *)val);
goto done;
--
2.12.2
^ permalink raw reply related
* [PATCH iproute2 master 3/4] pedit: Check for extended capability in protocol parser
From: Amir Vadai @ 2017-05-14 8:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Or Gerlitz, Amir Vadai
In-Reply-To: <20170514081746.9010-1-amir@vadai.me>
Do not allow using eth and udp header types if non-extended pedit kABI
is being used. Other protocol parsers already have this check.
Signed-off-by: Amir Vadai <amir@vadai.me>
---
tc/p_eth.c | 3 +++
tc/p_udp.c | 3 +++
2 files changed, 6 insertions(+)
diff --git a/tc/p_eth.c b/tc/p_eth.c
index ad3e28f80eb6..2d2f96ca2f0f 100644
--- a/tc/p_eth.c
+++ b/tc/p_eth.c
@@ -34,6 +34,9 @@ parse_eth(int *argc_p, char ***argv_p,
if (argc < 2)
return -1;
+ if (!sel->extended)
+ return -1;
+
tkey->htype = TCA_PEDIT_KEY_EX_HDR_TYPE_ETH;
if (strcmp(*argv, "type") == 0) {
diff --git a/tc/p_udp.c b/tc/p_udp.c
index a56a1b519254..3916d9586040 100644
--- a/tc/p_udp.c
+++ b/tc/p_udp.c
@@ -34,6 +34,9 @@ parse_udp(int *argc_p, char ***argv_p,
if (argc < 2)
return -1;
+ if (!sel->extended)
+ return -1;
+
tkey->htype = TCA_PEDIT_KEY_EX_HDR_TYPE_UDP;
if (strcmp(*argv, "sport") == 0) {
--
2.12.2
^ permalink raw reply related
* [PATCH iproute2 master 4/4] pedit: Introduce ipv6 support
From: Amir Vadai @ 2017-05-14 8:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Or Gerlitz, Amir Vadai
In-Reply-To: <20170514081746.9010-1-amir@vadai.me>
Add support for modifying IPv6 headers using pedit.
Signed-off-by: Amir Vadai <amir@vadai.me>
---
man/man8/tc-pedit.8 | 30 ++++++++++++++++++
tc/Makefile | 1 +
tc/m_pedit.c | 43 +++++++++++++++++++++++--
tc/p_ip.c | 17 +---------
tc/p_ip6.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 164 insertions(+), 18 deletions(-)
create mode 100644 tc/p_ip6.c
diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8
index 9c4d57b972cc..82d4217bc958 100644
--- a/man/man8/tc-pedit.8
+++ b/man/man8/tc-pedit.8
@@ -33,6 +33,8 @@ pedit - generic packet editor action
|
.BI ip " EX_IPHDR_FIELD"
|
+.BI ip6 " IP6HDR_FIELD"
+|
.BI tcp " TCPHDR_FIELD"
|
.BI udp " UDPHDR_FIELD"
@@ -55,6 +57,12 @@ pedit - generic packet editor action
.IR EX_IPHDR_FIELD " := { "
.BR ttl " }"
+
+.ti -8
+.IR IP6HDR_FIELD " := { "
+.BR src " | " dst " | " flow_lbl " | " payload_len " | " nexthdr " |"
+.BR hoplimit " }"
+
.ti -8
.IR TCPHDR_FIELD " := { "
.BR sport " | " dport " | " flags " }"
@@ -211,6 +219,25 @@ are:
.B ttl
.RE
.TP
+.BI ip6 " IP6HDR_FIELD"
+The supported keywords for
+.I IP6HDR_FIELD
+are:
+.RS
+.TP
+.B src
+.TQ
+.B dst
+.TQ
+.B flow_lbl
+.TQ
+.B payload_len
+.TQ
+.B nexthdr
+.TQ
+.B hoplimit
+.RE
+.TP
.BI tcp " TCPHDR_FIELD"
The supported keywords for
.I TCPHDR_FIELD
@@ -331,6 +358,9 @@ tc filter add dev eth0 parent ffff: u32 \\
action pedit ex munge ip dst set 192.168.1.199
tc filter add dev eth0 parent ffff: u32 \\
match ip sport 22 0xffff \\
+ action pedit ex munge ip6 dst set fe80::dacb:8aff:fec7:320e
+tc filter add dev eth0 parent ffff: u32 \\
+ match ip sport 22 0xffff \\
action pedit ex munge eth dst set 11:22:33:44:55:66
tc filter add dev eth0 parent ffff: u32 \\
match ip dport 23 0xffff \\
diff --git a/tc/Makefile b/tc/Makefile
index 446a11391ad7..9a6bb1ddea57 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -53,6 +53,7 @@ TCMODULES += m_bpf.o
TCMODULES += m_tunnel_key.o
TCMODULES += m_sample.o
TCMODULES += p_ip.o
+TCMODULES += p_ip6.o
TCMODULES += p_icmp.o
TCMODULES += p_eth.o
TCMODULES += p_tcp.o
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 9b74c965932e..dfa6b2c4835e 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -257,6 +257,32 @@ static int pack_mac(struct m_pedit_sel *sel, struct m_pedit_key *tkey,
return ret;
}
+static int pack_ipv6(struct m_pedit_sel *sel, struct m_pedit_key *tkey,
+ __u32 *ipv6)
+{
+ int ret = 0;
+ int i;
+
+ if (tkey->off & 0x3) {
+ fprintf(stderr,
+ "pack_ipv6: IPv6 offsets must begin in 32bit boundaries\n");
+ return -1;
+ }
+
+ for (i = 0; i < 4; i++) {
+ tkey->mask = 0;
+ tkey->val = ntohl(ipv6[i]);
+
+ ret = pack_key32(~0, sel, tkey);
+ if (ret)
+ return ret;
+
+ tkey->off += 4;
+ }
+
+ return 0;
+}
+
int parse_val(int *argc_p, char ***argv_p, __u32 *val, int type)
{
int argc = *argc_p;
@@ -281,8 +307,16 @@ int parse_val(int *argc_p, char ***argv_p, __u32 *val, int type)
return 0;
}
- if (type == TIPV6)
- return -1; /* not implemented yet */
+ if (type == TIPV6) {
+ inet_prefix addr;
+
+ if (get_prefix_1(&addr, *argv, AF_INET6))
+ return -1;
+
+ memcpy(val, addr.data, addr.bytelen);
+
+ return 0;
+ }
if (type == TMAC) {
#define MAC_ALEN 6
@@ -364,6 +398,11 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
goto done;
}
+ if (type == TIPV6) {
+ res = pack_ipv6(sel, tkey, val);
+ goto done;
+ }
+
tkey->val = *v;
tkey->mask = *m;
diff --git a/tc/p_ip.c b/tc/p_ip.c
index 22fe6505e427..0272a6eaaf48 100644
--- a/tc/p_ip.c
+++ b/tc/p_ip.c
@@ -1,5 +1,5 @@
/*
- * m_pedit.c packet editor: IPV4/6 header
+ * p_ip.c packet editor: IPV4 header
*
* This program is free software; you can distribute it and/or
* modify it under the terms of the GNU General Public License
@@ -156,23 +156,8 @@ done:
return res;
}
-static int
-parse_ip6(int *argc_p, char ***argv_p,
- struct m_pedit_sel *sel, struct m_pedit_key *tkey)
-{
- int res = -1;
- return res;
-}
-
struct m_pedit_util p_pedit_ip = {
NULL,
"ip",
parse_ip,
};
-
-
-struct m_pedit_util p_pedit_ip6 = {
- NULL,
- "ip6",
- parse_ip6,
-};
diff --git a/tc/p_ip6.c b/tc/p_ip6.c
new file mode 100644
index 000000000000..a4824bda90e8
--- /dev/null
+++ b/tc/p_ip6.c
@@ -0,0 +1,91 @@
+/*
+ * p_ip6.c packet editor: IPV6 header
+ *
+ * This program is free software; you can distribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Amir Vadai <amir@vadai.me>
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+#include "utils.h"
+#include "tc_util.h"
+#include "m_pedit.h"
+
+static int
+parse_ip6(int *argc_p, char ***argv_p,
+ struct m_pedit_sel *sel, struct m_pedit_key *tkey)
+{
+ int res = -1;
+ int argc = *argc_p;
+ char **argv = *argv_p;
+
+ if (argc < 2)
+ return -1;
+
+ if (!sel->extended)
+ return -1;
+
+ tkey->htype = TCA_PEDIT_KEY_EX_HDR_TYPE_IP6;
+
+ if (strcmp(*argv, "src") == 0) {
+ NEXT_ARG();
+ tkey->off = 8;
+ res = parse_cmd(&argc, &argv, 16, TIPV6, RU32, sel, tkey);
+ goto done;
+ }
+ if (strcmp(*argv, "dst") == 0) {
+ NEXT_ARG();
+ tkey->off = 24;
+ res = parse_cmd(&argc, &argv, 16, TIPV6, RU32, sel, tkey);
+ goto done;
+ }
+ if (strcmp(*argv, "flow_lbl") == 0) {
+ NEXT_ARG();
+ tkey->off = 0;
+ res = parse_cmd(&argc, &argv, 4, TU32, 0x0007ffff, sel, tkey);
+ goto done;
+ }
+ if (strcmp(*argv, "payload_len") == 0) {
+ NEXT_ARG();
+ tkey->off = 4;
+ res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+ goto done;
+ }
+ if (strcmp(*argv, "nexthdr") == 0) {
+ NEXT_ARG();
+ tkey->off = 6;
+ res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+ goto done;
+ }
+ if (strcmp(*argv, "hoplimit") == 0) {
+ NEXT_ARG();
+ tkey->off = 7;
+ res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+ goto done;
+ }
+
+ return -1;
+
+done:
+ *argc_p = argc;
+ *argv_p = argv;
+ return res;
+}
+
+struct m_pedit_util p_pedit_ip6 = {
+ NULL,
+ "ipv6",
+ parse_ip6,
+};
--
2.12.2
^ permalink raw reply related
* Re: [net 1/6] net/mlx5e: Use a spinlock to synchronize statistics
From: Saeed Mahameed @ 2017-05-14 8:52 UTC (permalink / raw)
To: David Miller; +Cc: Saeed Mahameed, Linux Netdev List, Gal Pressman, Kernel Team
In-Reply-To: <20170512.121354.1583292892374462482.davem@davemloft.net>
On Fri, May 12, 2017 at 7:13 PM, David Miller <davem@davemloft.net> wrote:
> From: Saeed Mahameed <saeedm@mellanox.com>
> Date: Fri, 12 May 2017 14:56:45 +0300
>
>> From: Gal Pressman <galp@mellanox.com>
>>
>> Add a spinlock to prevent races when querying statistics, for example
>> querying counters in the middle of a non atomic memcpy() operation in
>> mlx5e_update_stats().
>>
>> This RW lock should be held when accessing priv->stats, to prevent other
>> reads/writes.
>>
>> Fixes: 9218b44dcc05 ("net/mlx5e: Statistics handling refactoring")
>> Signed-off-by: Gal Pressman <galp@mellanox.com>
>> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
>> Cc: kernel-team@fb.com
>> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>
> This is overkill, and that rwlock is going to show up in perf for some
> workloads.
>
> Furthermore, two kzalloc()'s for a single state update operation?
> That's not reasonable either.
>
Hi Dave,
Well, the idea of the patch is to minimize the stats update to a
single safe copy operation under stats_lock
for that we need a temp buffer to store stats FW commands output, and
then safely -under stats_lock- copy it to
the buffer ethtool is going to report.
I agree, it is really ridiculous that we allocate/free a couple of
buffers on each update_stats operations, regardless of this patch.
Is it ok if we use a temp buffer under netdev_priv for such usages or
even use kmemcache ?
> Use a seqlock, which is the primitive for handling this kind of
> situation cheaply, and adds no atomics to the read path.
>
Will change this. Thanks for the tip.
I will drop this patch for now and I will resend it later once Gal
addresses all of the above comments.
Thanks,
Saeed.
^ permalink raw reply
* Re: [PATCH v2 net-next 3/5] dsa: add DSA switch driver for Microchip KSZ9477
From: Sergei Shtylyov @ 2017-05-14 9:13 UTC (permalink / raw)
To: Andrew Lunn, Woojung.Huh
Cc: f.fainelli, vivien.didelot, netdev, davem, UNGLinuxDriver
In-Reply-To: <20170513145609.GD14058@lunn.ch>
Hello!
On 5/13/2017 5:56 PM, Andrew Lunn wrote:
[...]
>> +static int ksz_reset_switch(struct dsa_switch *ds)
>> +{
>> + struct ksz_device *dev = ds->priv;
>> + u8 data8;
>> + u16 data16;
>> + u32 data32;
>> +
>
> ...
>
>> +
>> + memset(dev->vlan_cache, 0, sizeof(*dev->vlan_cache) * dev->num_vlans);
>> +
>> + return 0;
>> +}
>
>> +static int ksz_setup(struct dsa_switch *ds)
>> +{
>> + struct ksz_device *dev = ds->priv;
>> + int ret = 0;
>> +
>> + dev->vlan_cache = devm_kmalloc_array(dev->dev,
>> + sizeof(struct vlan_table),
>> + dev->num_vlans, GFP_KERNEL);
>
> You should check, but i think devm_kmalloc_array sets the allocated
> memory to 0.
No. Else there would be no need for it, since kcalloc() is a function that
allocates the arrays and zeroes them.
> So i don't think you need the memset. If it is needed, i
> would move it here, after the check the allocation is successful.
If it could be done here, kcalloc() should be used.
[...]
MBR, Sergei
^ permalink raw reply
* [PATCH net v2] qed: Fix uninitialized data in aRFS infrastructure
From: Yuval Mintz @ 2017-05-14 9:21 UTC (permalink / raw)
To: arnd, davem, netdev; +Cc: Yuval Mintz
Current memset is using incorrect type of variable, causing the
upper-half of the strucutre to be left uninitialized and causing:
ethernet/qlogic/qed/qed_init_fw_funcs.c: In function 'qed_set_rfs_mode_disable':
ethernet/qlogic/qed/qed_init_fw_funcs.c:993:3: error: '*((void *)&ramline+4)' is used uninitialized in this function [-Werror=uninitialized]
Fixes: d51e4af5c209 ("qed: aRFS infrastructure support")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Yuval Mintz <Yuval.Mintz@cavium.com>
---
Hi Arnd,
I don't see the original warning you've reported [probably due to
compiler differences], so other than via code reading I couldn't
actually test this. Can you please compile-test this one?
As mentioned in the original thread, I'll send the style-change
later to net-next.
Dave - please consider applying this to `net'.
Thanks,
Yuval
---
drivers/net/ethernet/qlogic/qed/qed_init_fw_funcs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_init_fw_funcs.c b/drivers/net/ethernet/qlogic/qed/qed_init_fw_funcs.c
index 67200c5..0a8fde6 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_init_fw_funcs.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_init_fw_funcs.c
@@ -983,7 +983,7 @@ void qed_set_rfs_mode_disable(struct qed_hwfn *p_hwfn,
memset(&camline, 0, sizeof(union gft_cam_line_union));
qed_wr(p_hwfn, p_ptt, PRS_REG_GFT_CAM + CAM_LINE_SIZE * pf_id,
camline.cam_line_mapped.camline);
- memset(&ramline, 0, sizeof(union gft_cam_line_union));
+ memset(&ramline, 0, sizeof(ramline));
for (i = 0; i < RAM_LINE_SIZE / REG_SIZE; i++) {
u32 hw_addr = PRS_REG_GFT_PROFILE_MASK_RAM;
--
1.9.3
^ permalink raw reply related
* [pull request][net V2 0/5] Mellanox, mlx5 fixes 2017-05-12
From: Saeed Mahameed @ 2017-05-14 10:43 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Saeed Mahameed
Hi Dave,
This series contains some mlx5 fixes for net.
Please pull and let me know if there's any problem.
For -stable:
("net/mlx5e: Fix ethtool pause support and advertise reporting") kernels >= 4.8
("net/mlx5e: Use the correct pause values for ethtool advertising") kernels >= 4.8
v1->v2:
Dropped statistics spinlock patch, it needs some extra work.
Thanks,
Saeed.
The following changes since commit 1c4d5f51a812a82de97beee24f48ed05c65ebda5:
vmxnet3: ensure that adapter is in proper state during force_close (2017-05-12 12:23:52 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git tags/mlx5-fixes-2017-05-12-V2
for you to fetch changes up to 508541146af18e43072e41a31aa62fac2b01aac1:
net/mlx5: Use underlay QPN from the root name space (2017-05-14 13:33:45 +0300)
----------------------------------------------------------------
mlx5-fixes-2017-05-12
Misc fixes for mlx5 driver
----------------------------------------------------------------
Gal Pressman (2):
net/mlx5e: Use the correct pause values for ethtool advertising
net/mlx5e: Fix ethtool pause support and advertise reporting
Saeed Mahameed (2):
net/mlx5e: Fix setup TC ndo
net/mlx5e: IPoIB, Only support regular RQ for now
Yishai Hadas (1):
net/mlx5: Use underlay QPN from the root name space
drivers/net/ethernet/mellanox/mlx5/core/en.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 9 +++++---
drivers/net/ethernet/mellanox/mlx5/core/en_fs.c | 5 ++---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c | 9 +++-----
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.h | 3 ++-
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 25 ++++++++++++++++++----
drivers/net/ethernet/mellanox/mlx5/core/fs_core.h | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/ipoib.c | 11 ++++++++--
include/linux/mlx5/fs.h | 4 +++-
10 files changed, 49 insertions(+), 23 deletions(-)
^ permalink raw reply
* [net V2 1/5] net/mlx5e: Use the correct pause values for ethtool advertising
From: Saeed Mahameed @ 2017-05-14 10:43 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Gal Pressman, kernel-team, Saeed Mahameed
In-Reply-To: <20170514104311.2081-1-saeedm@mellanox.com>
From: Gal Pressman <galp@mellanox.com>
Query the operational pause from firmware (PFCC register) instead of
always passing zeros.
Fixes: 665bc53969d7 ("net/mlx5e: Use new ethtool get/set link ksettings API")
Signed-off-by: Gal Pressman <galp@mellanox.com>
Cc: kernel-team@fb.com
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index ce7b09d72ff6..d60e681b443e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -849,6 +849,8 @@ static int mlx5e_get_link_ksettings(struct net_device *netdev,
struct mlx5e_priv *priv = netdev_priv(netdev);
struct mlx5_core_dev *mdev = priv->mdev;
u32 out[MLX5_ST_SZ_DW(ptys_reg)] = {0};
+ u32 rx_pause = 0;
+ u32 tx_pause = 0;
u32 eth_proto_cap;
u32 eth_proto_admin;
u32 eth_proto_lp;
@@ -871,11 +873,13 @@ static int mlx5e_get_link_ksettings(struct net_device *netdev,
an_disable_admin = MLX5_GET(ptys_reg, out, an_disable_admin);
an_status = MLX5_GET(ptys_reg, out, an_status);
+ mlx5_query_port_pause(mdev, &rx_pause, &tx_pause);
+
ethtool_link_ksettings_zero_link_mode(link_ksettings, supported);
ethtool_link_ksettings_zero_link_mode(link_ksettings, advertising);
get_supported(eth_proto_cap, link_ksettings);
- get_advertising(eth_proto_admin, 0, 0, link_ksettings);
+ get_advertising(eth_proto_admin, tx_pause, rx_pause, link_ksettings);
get_speed_duplex(netdev, eth_proto_oper, link_ksettings);
eth_proto_oper = eth_proto_oper ? eth_proto_oper : eth_proto_cap;
--
2.11.0
^ permalink raw reply related
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