* Re: [PATCH net-next 4/4] net: dsa: lan9303: MDIO access phy registers directly
From: Andrew Lunn @ 2017-07-28 15:24 UTC (permalink / raw)
To: Egil Hjelmeland; +Cc: vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
In-Reply-To: <20170728151157.28983-5-privat@egil-hjelmeland.no>
On Fri, Jul 28, 2017 at 05:11:57PM +0200, Egil Hjelmeland wrote:
> Indirect access (PMI) to phy register only work in I2C mode. In
> MDIO mode phy registers must be accessed directly. Introduced
> struct lan9303_phy_ops to handle the two modes.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: dsa: lan9303: Renamed indirect phy access functions
From: Andrew Lunn @ 2017-07-28 15:22 UTC (permalink / raw)
To: Egil Hjelmeland; +Cc: vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
In-Reply-To: <20170728151157.28983-4-privat@egil-hjelmeland.no>
On Fri, Jul 28, 2017 at 05:11:56PM +0200, Egil Hjelmeland wrote:
> Preparing for the following fix of MDIO phy access:
>
> Renamed functions that access PHY 1 and 2 indirectly through PMI
> registers.
>
> lan9303_port_phy_reg_wait_for_completion() to
> lan9303_indirect_phy_wait_for_completion()
>
> lan9303_port_phy_reg_read() to
> lan9303_indirect_phy_read()
>
> lan9303_port_phy_reg_write() to
> lan9303_indirect_phy_write()
>
> Also changed "val" parameter of lan9303_indirect_phy_write() to u16,
> for clarity.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 2/4] net: dsa: lan9303: Multiply by 4 to get MDIO register
From: Andrew Lunn @ 2017-07-28 15:20 UTC (permalink / raw)
To: Egil Hjelmeland; +Cc: vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
In-Reply-To: <20170728151157.28983-3-privat@egil-hjelmeland.no>
On Fri, Jul 28, 2017 at 05:11:55PM +0200, Egil Hjelmeland wrote:
> lan9303_mdio_write()/_read() must multiply register number by 4 to get
> offset.
>
> Added some commments to the register definitions.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: dsa: lan9303: Fix lan9303_detect_phy_setup() for MDIO
From: Andrew Lunn @ 2017-07-28 15:19 UTC (permalink / raw)
To: Egil Hjelmeland; +Cc: vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
In-Reply-To: <20170728151157.28983-2-privat@egil-hjelmeland.no>
On Fri, Jul 28, 2017 at 05:11:54PM +0200, Egil Hjelmeland wrote:
> Handle that MDIO read with no response return 0xffff.
>
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v11 1/4] net netlink: Add new type NLA_BITFIELD_32
From: David Ahern @ 2017-07-28 15:13 UTC (permalink / raw)
To: Jamal Hadi Salim, davem
Cc: netdev, jiri, xiyou.wangcong, eric.dumazet, mrv, simon.horman,
alex.aring
In-Reply-To: <b82ef71c-3231-fa71-44f5-8882750822d9@mojatatu.com>
On 7/28/17 9:04 AM, Jamal Hadi Salim wrote:
>
> Kernel side checking for device ifindex must know what a device
> ifindex means.
> That doesnt disqualify that the generic code checks that it
> is of the same size as a signed 32b, etc. That is generic
> stuff that can be factored out.
>
> In this case:
> Checking for whether bits selected are in the allowed range
> that the kernel understands, that the bit value are set in
> the right bit position, that the bits set in the correct bit
> value position are also selected in the transaction.
> That is generic code (which the content validation does).
Create a helper function then. It's the validation of attribute content
in 2 places that I object to. 1 attribute is validated in 1 place
(generic infra for this bitfield attribute), the others are validated in
line. Asymmetric validations is not a good design.
^ permalink raw reply
* Re: [PATCH] Cipso: cipso_v4_optptr enter infinite loop
From: Paul Moore @ 2017-07-28 15:12 UTC (permalink / raw)
To: Yujuan Qi
Cc: David S. Miller, Casey Schaufler, netdev, linux-mediatek,
linux-kernel, Ryder Lee
In-Reply-To: <1501241408-19129-1-git-send-email-yujuan.qi@mediatek.com>
On Fri, Jul 28, 2017 at 7:30 AM, Yujuan Qi <yujuan.qi@mediatek.com> wrote:
> From: "yujuan.qi" <yujuan.qi@mediatek.com>
>
> in for(),if((optlen > 0) && (optptr[1] == 0)), enter infinite loop.
>
> Test: receive a packet which the ip length > 20 and the first byte of ip option is 0, produce this issue
>
> Signed-off-by: yujuan.qi <yujuan.qi@mediatek.com>
> ---
> net/ipv4/cipso_ipv4.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index ae20616..b3d97c7 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -1528,6 +1528,8 @@ unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
> taglen = optptr[1];
> optlen -= taglen;
> optptr += taglen;
> + if (!taglen)
> + break;
> }
>
> return NULL;
Thanks for catching this and submitting a patch. Unfortunately
checking taglen/optptr[1] isn't going to help with the NOP and EOL
options as they are single byte options. I think a for-loop like the
following, or something similar, is probably the better solution:
for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 0; ) {
switch (optptr[0]) {
case IPOPT_CIPSO:
return optptr;
case IPOPT_END:
return NULL;
case IPOPT_NOOP:
taglen = 1;
break;
default:
taglen = optptr[1];
}
optlen -= taglen;
optptr += taglen;
}
--
paul moore
security @ redhat
^ permalink raw reply
* [PATCH net-next 0/4] net: dsa: lan9303: Fix MDIO issues.
From: Egil Hjelmeland @ 2017-07-28 15:11 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
Cc: Egil Hjelmeland
This series fix the MDIO interface for the lan9303 DSA driver.
Bugs found after testing on actual HW.
This series is extracted from the first patch of my first large
series. Significant changes from that version are:
- use mdiobus_write_nested, mdiobus_read_nested.
- EXPORT lan9303_indirect_phy_ops
Unfortunately I do not have access to i2c based system for
testing.
Egil Hjelmeland (4):
net: dsa: lan9303: Fix lan9303_detect_phy_setup() for MDIO
net: dsa: lan9303: Multiply by 4 to get MDIO register
net: dsa: lan9303: Renamed indirect phy access functions
net: dsa: lan9303: MDIO access phy registers directly
drivers/net/dsa/lan9303-core.c | 43 +++++++++++++++++++++++++++---------------
drivers/net/dsa/lan9303.h | 11 +++++++++++
drivers/net/dsa/lan9303_i2c.c | 2 ++
drivers/net/dsa/lan9303_mdio.c | 23 ++++++++++++++++++++++
4 files changed, 64 insertions(+), 15 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH net-next 4/4] net: dsa: lan9303: MDIO access phy registers directly
From: Egil Hjelmeland @ 2017-07-28 15:11 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
Cc: Egil Hjelmeland
In-Reply-To: <20170728151157.28983-1-privat@egil-hjelmeland.no>
Indirect access (PMI) to phy register only work in I2C mode. In
MDIO mode phy registers must be accessed directly. Introduced
struct lan9303_phy_ops to handle the two modes.
Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
drivers/net/dsa/lan9303-core.c | 20 +++++++++++++-------
drivers/net/dsa/lan9303.h | 11 +++++++++++
drivers/net/dsa/lan9303_i2c.c | 2 ++
drivers/net/dsa/lan9303_mdio.c | 21 +++++++++++++++++++++
4 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 9427c3b0ced2..9910fc9d5dd6 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -334,6 +334,12 @@ static int lan9303_indirect_phy_write(struct lan9303 *chip, int addr,
return ret;
}
+const struct lan9303_phy_ops lan9303_indirect_phy_ops = {
+ .phy_read = lan9303_indirect_phy_read,
+ .phy_write = lan9303_indirect_phy_write,
+};
+EXPORT_SYMBOL(lan9303_indirect_phy_ops);
+
static int lan9303_switch_wait_for_completion(struct lan9303 *chip)
{
int ret, i;
@@ -435,7 +441,7 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
* 0x0000, which means 'phy_addr_sel_strap' is 1 and the IDs are 1-2-3.
* 0xffff is returned on MDIO read with no response.
*/
- reg = lan9303_indirect_phy_read(chip, 3, MII_LAN911X_SPECIAL_MODES);
+ reg = chip->ops->phy_read(chip, 3, MII_LAN911X_SPECIAL_MODES);
if (reg < 0) {
dev_err(chip->dev, "Failed to detect phy config: %d\n", reg);
return reg;
@@ -726,7 +732,7 @@ static int lan9303_phy_read(struct dsa_switch *ds, int phy, int regnum)
if (phy > phy_base + 2)
return -ENODEV;
- return lan9303_indirect_phy_read(chip, phy, regnum);
+ return chip->ops->phy_read(chip, phy, regnum);
}
static int lan9303_phy_write(struct dsa_switch *ds, int phy, int regnum,
@@ -740,7 +746,7 @@ static int lan9303_phy_write(struct dsa_switch *ds, int phy, int regnum,
if (phy > phy_base + 2)
return -ENODEV;
- return lan9303_indirect_phy_write(chip, phy, regnum, val);
+ return chip->ops->phy_write(chip, phy, regnum, val);
}
static int lan9303_port_enable(struct dsa_switch *ds, int port,
@@ -773,13 +779,13 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port,
switch (port) {
case 1:
lan9303_disable_packet_processing(chip, LAN9303_PORT_1_OFFSET);
- lan9303_indirect_phy_write(chip, chip->phy_addr_sel_strap + 1,
- MII_BMCR, BMCR_PDOWN);
+ lan9303_phy_write(ds, chip->phy_addr_sel_strap + 1,
+ MII_BMCR, BMCR_PDOWN);
break;
case 2:
lan9303_disable_packet_processing(chip, LAN9303_PORT_2_OFFSET);
- lan9303_indirect_phy_write(chip, chip->phy_addr_sel_strap + 2,
- MII_BMCR, BMCR_PDOWN);
+ lan9303_phy_write(ds, chip->phy_addr_sel_strap + 2,
+ MII_BMCR, BMCR_PDOWN);
break;
default:
dev_dbg(chip->dev,
diff --git a/drivers/net/dsa/lan9303.h b/drivers/net/dsa/lan9303.h
index d1512dad2d90..4d8be555ff4d 100644
--- a/drivers/net/dsa/lan9303.h
+++ b/drivers/net/dsa/lan9303.h
@@ -2,6 +2,15 @@
#include <linux/device.h>
#include <net/dsa.h>
+struct lan9303;
+
+struct lan9303_phy_ops {
+ /* PHY 1 and 2 access*/
+ int (*phy_read)(struct lan9303 *chip, int port, int regnum);
+ int (*phy_write)(struct lan9303 *chip, int port,
+ int regnum, u16 val);
+};
+
struct lan9303 {
struct device *dev;
struct regmap *regmap;
@@ -11,9 +20,11 @@ struct lan9303 {
bool phy_addr_sel_strap;
struct dsa_switch *ds;
struct mutex indirect_mutex; /* protect indexed register access */
+ const struct lan9303_phy_ops *ops;
};
extern const struct regmap_access_table lan9303_register_set;
+extern const struct lan9303_phy_ops lan9303_indirect_phy_ops;
int lan9303_probe(struct lan9303 *chip, struct device_node *np);
int lan9303_remove(struct lan9303 *chip);
diff --git a/drivers/net/dsa/lan9303_i2c.c b/drivers/net/dsa/lan9303_i2c.c
index ab3ce0da5071..24ec20f7f444 100644
--- a/drivers/net/dsa/lan9303_i2c.c
+++ b/drivers/net/dsa/lan9303_i2c.c
@@ -63,6 +63,8 @@ static int lan9303_i2c_probe(struct i2c_client *client,
i2c_set_clientdata(client, sw_dev);
sw_dev->chip.dev = &client->dev;
+ sw_dev->chip.ops = &lan9303_indirect_phy_ops;
+
ret = lan9303_probe(&sw_dev->chip, client->dev.of_node);
if (ret != 0)
return ret;
diff --git a/drivers/net/dsa/lan9303_mdio.c b/drivers/net/dsa/lan9303_mdio.c
index 2db7970fc88c..fc16668a487f 100644
--- a/drivers/net/dsa/lan9303_mdio.c
+++ b/drivers/net/dsa/lan9303_mdio.c
@@ -67,6 +67,25 @@ static int lan9303_mdio_read(void *ctx, uint32_t reg, uint32_t *val)
return 0;
}
+int lan9303_mdio_phy_write(struct lan9303 *chip, int phy, int reg, u16 val)
+{
+ struct lan9303_mdio *sw_dev = dev_get_drvdata(chip->dev);
+
+ return mdiobus_write_nested(sw_dev->device->bus, phy, reg, val);
+}
+
+int lan9303_mdio_phy_read(struct lan9303 *chip, int phy, int reg)
+{
+ struct lan9303_mdio *sw_dev = dev_get_drvdata(chip->dev);
+
+ return mdiobus_read_nested(sw_dev->device->bus, phy, reg);
+}
+
+static const struct lan9303_phy_ops lan9303_mdio_phy_ops = {
+ .phy_read = lan9303_mdio_phy_read,
+ .phy_write = lan9303_mdio_phy_write,
+};
+
static const struct regmap_config lan9303_mdio_regmap_config = {
.reg_bits = 8,
.val_bits = 32,
@@ -108,6 +127,8 @@ static int lan9303_mdio_probe(struct mdio_device *mdiodev)
dev_set_drvdata(&mdiodev->dev, sw_dev);
sw_dev->chip.dev = &mdiodev->dev;
+ sw_dev->chip.ops = &lan9303_mdio_phy_ops;
+
ret = lan9303_probe(&sw_dev->chip, mdiodev->dev.of_node);
if (ret != 0)
return ret;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 3/4] net: dsa: lan9303: Renamed indirect phy access functions
From: Egil Hjelmeland @ 2017-07-28 15:11 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
Cc: Egil Hjelmeland
In-Reply-To: <20170728151157.28983-1-privat@egil-hjelmeland.no>
Preparing for the following fix of MDIO phy access:
Renamed functions that access PHY 1 and 2 indirectly through PMI
registers.
lan9303_port_phy_reg_wait_for_completion() to
lan9303_indirect_phy_wait_for_completion()
lan9303_port_phy_reg_read() to
lan9303_indirect_phy_read()
lan9303_port_phy_reg_write() to
lan9303_indirect_phy_write()
Also changed "val" parameter of lan9303_indirect_phy_write() to u16,
for clarity.
Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
drivers/net/dsa/lan9303-core.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 96ebeb9bd59a..9427c3b0ced2 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -248,7 +248,7 @@ static int lan9303_virt_phy_reg_write(struct lan9303 *chip, int regnum, u16 val)
return regmap_write(chip->regmap, LAN9303_VIRT_PHY_BASE + regnum, val);
}
-static int lan9303_port_phy_reg_wait_for_completion(struct lan9303 *chip)
+static int lan9303_indirect_phy_wait_for_completion(struct lan9303 *chip)
{
int ret, i;
u32 reg;
@@ -268,7 +268,7 @@ static int lan9303_port_phy_reg_wait_for_completion(struct lan9303 *chip)
return -EIO;
}
-static int lan9303_port_phy_reg_read(struct lan9303 *chip, int addr, int regnum)
+static int lan9303_indirect_phy_read(struct lan9303 *chip, int addr, int regnum)
{
int ret;
u32 val;
@@ -278,7 +278,7 @@ static int lan9303_port_phy_reg_read(struct lan9303 *chip, int addr, int regnum)
mutex_lock(&chip->indirect_mutex);
- ret = lan9303_port_phy_reg_wait_for_completion(chip);
+ ret = lan9303_indirect_phy_wait_for_completion(chip);
if (ret)
goto on_error;
@@ -287,7 +287,7 @@ static int lan9303_port_phy_reg_read(struct lan9303 *chip, int addr, int regnum)
if (ret)
goto on_error;
- ret = lan9303_port_phy_reg_wait_for_completion(chip);
+ ret = lan9303_indirect_phy_wait_for_completion(chip);
if (ret)
goto on_error;
@@ -305,8 +305,8 @@ static int lan9303_port_phy_reg_read(struct lan9303 *chip, int addr, int regnum)
return ret;
}
-static int lan9303_phy_reg_write(struct lan9303 *chip, int addr, int regnum,
- unsigned int val)
+static int lan9303_indirect_phy_write(struct lan9303 *chip, int addr,
+ int regnum, u16 val)
{
int ret;
u32 reg;
@@ -317,7 +317,7 @@ static int lan9303_phy_reg_write(struct lan9303 *chip, int addr, int regnum,
mutex_lock(&chip->indirect_mutex);
- ret = lan9303_port_phy_reg_wait_for_completion(chip);
+ ret = lan9303_indirect_phy_wait_for_completion(chip);
if (ret)
goto on_error;
@@ -435,7 +435,7 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
* 0x0000, which means 'phy_addr_sel_strap' is 1 and the IDs are 1-2-3.
* 0xffff is returned on MDIO read with no response.
*/
- reg = lan9303_port_phy_reg_read(chip, 3, MII_LAN911X_SPECIAL_MODES);
+ reg = lan9303_indirect_phy_read(chip, 3, MII_LAN911X_SPECIAL_MODES);
if (reg < 0) {
dev_err(chip->dev, "Failed to detect phy config: %d\n", reg);
return reg;
@@ -726,7 +726,7 @@ static int lan9303_phy_read(struct dsa_switch *ds, int phy, int regnum)
if (phy > phy_base + 2)
return -ENODEV;
- return lan9303_port_phy_reg_read(chip, phy, regnum);
+ return lan9303_indirect_phy_read(chip, phy, regnum);
}
static int lan9303_phy_write(struct dsa_switch *ds, int phy, int regnum,
@@ -740,7 +740,7 @@ static int lan9303_phy_write(struct dsa_switch *ds, int phy, int regnum,
if (phy > phy_base + 2)
return -ENODEV;
- return lan9303_phy_reg_write(chip, phy, regnum, val);
+ return lan9303_indirect_phy_write(chip, phy, regnum, val);
}
static int lan9303_port_enable(struct dsa_switch *ds, int port,
@@ -773,13 +773,13 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port,
switch (port) {
case 1:
lan9303_disable_packet_processing(chip, LAN9303_PORT_1_OFFSET);
- lan9303_phy_reg_write(chip, chip->phy_addr_sel_strap + 1,
- MII_BMCR, BMCR_PDOWN);
+ lan9303_indirect_phy_write(chip, chip->phy_addr_sel_strap + 1,
+ MII_BMCR, BMCR_PDOWN);
break;
case 2:
lan9303_disable_packet_processing(chip, LAN9303_PORT_2_OFFSET);
- lan9303_phy_reg_write(chip, chip->phy_addr_sel_strap + 2,
- MII_BMCR, BMCR_PDOWN);
+ lan9303_indirect_phy_write(chip, chip->phy_addr_sel_strap + 2,
+ MII_BMCR, BMCR_PDOWN);
break;
default:
dev_dbg(chip->dev,
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 2/4] net: dsa: lan9303: Multiply by 4 to get MDIO register
From: Egil Hjelmeland @ 2017-07-28 15:11 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
Cc: Egil Hjelmeland
In-Reply-To: <20170728151157.28983-1-privat@egil-hjelmeland.no>
lan9303_mdio_write()/_read() must multiply register number by 4 to get
offset.
Added some commments to the register definitions.
Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
drivers/net/dsa/lan9303-core.c | 6 ++++++
drivers/net/dsa/lan9303_mdio.c | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 9d0ab77edb4a..96ebeb9bd59a 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -20,6 +20,9 @@
#include "lan9303.h"
+/* 13.2 System Control and Status Registers
+ * Multiply register number by 4 to get address offset.
+ */
#define LAN9303_CHIP_REV 0x14
# define LAN9303_CHIP_ID 0x9303
#define LAN9303_IRQ_CFG 0x15
@@ -53,6 +56,9 @@
#define LAN9303_VIRT_PHY_BASE 0x70
#define LAN9303_VIRT_SPECIAL_CTRL 0x77
+/*13.4 Switch Fabric Control and Status Registers
+ * Accessed indirectly via SWITCH_CSR_CMD, SWITCH_CSR_DATA.
+ */
#define LAN9303_SW_DEV_ID 0x0000
#define LAN9303_SW_RESET 0x0001
#define LAN9303_SW_RESET_RESET BIT(0)
diff --git a/drivers/net/dsa/lan9303_mdio.c b/drivers/net/dsa/lan9303_mdio.c
index 93c36c0541cf..2db7970fc88c 100644
--- a/drivers/net/dsa/lan9303_mdio.c
+++ b/drivers/net/dsa/lan9303_mdio.c
@@ -40,6 +40,7 @@ static int lan9303_mdio_write(void *ctx, uint32_t reg, uint32_t val)
{
struct lan9303_mdio *sw_dev = (struct lan9303_mdio *)ctx;
+ reg <<= 2; /* reg num to offset */
mutex_lock(&sw_dev->device->bus->mdio_lock);
lan9303_mdio_real_write(sw_dev->device, reg, val & 0xffff);
lan9303_mdio_real_write(sw_dev->device, reg + 2, (val >> 16) & 0xffff);
@@ -57,6 +58,7 @@ static int lan9303_mdio_read(void *ctx, uint32_t reg, uint32_t *val)
{
struct lan9303_mdio *sw_dev = (struct lan9303_mdio *)ctx;
+ reg <<= 2; /* reg num to offset */
mutex_lock(&sw_dev->device->bus->mdio_lock);
*val = lan9303_mdio_real_read(sw_dev->device, reg);
*val |= (lan9303_mdio_real_read(sw_dev->device, reg + 2) << 16);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 1/4] net: dsa: lan9303: Fix lan9303_detect_phy_setup() for MDIO
From: Egil Hjelmeland @ 2017-07-28 15:11 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel, kernel
Cc: Egil Hjelmeland
In-Reply-To: <20170728151157.28983-1-privat@egil-hjelmeland.no>
Handle that MDIO read with no response return 0xffff.
Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
drivers/net/dsa/lan9303-core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index cd76e61f1fca..9d0ab77edb4a 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -427,6 +427,7 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
* Special reg 18 of phy 3 reads as 0x0000, if 'phy_addr_sel_strap' is 0
* and the IDs are 0-1-2, else it contains something different from
* 0x0000, which means 'phy_addr_sel_strap' is 1 and the IDs are 1-2-3.
+ * 0xffff is returned on MDIO read with no response.
*/
reg = lan9303_port_phy_reg_read(chip, 3, MII_LAN911X_SPECIAL_MODES);
if (reg < 0) {
@@ -434,7 +435,7 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
return reg;
}
- if (reg != 0)
+ if ((reg != 0) && (reg != 0xffff))
chip->phy_addr_sel_strap = 1;
else
chip->phy_addr_sel_strap = 0;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next 3/3] tap: XDP support
From: Daniel Borkmann @ 2017-07-28 15:11 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang
Cc: Jakub Kicinski, davem, netdev, linux-kernel
In-Reply-To: <20170728064603-mutt-send-email-mst@kernel.org>
On 07/28/2017 05:46 AM, Michael S. Tsirkin wrote:
> On Fri, Jul 28, 2017 at 11:28:54AM +0800, Jason Wang wrote:
>>>> + old_prog = rtnl_dereference(tun->xdp_prog);
>>>> + if (old_prog)
>>>> + bpf_prog_put(old_prog);
>>>> + rcu_assign_pointer(tun->xdp_prog, prog);
>>> Is this OK? Could this lead to the program getting freed and then
>>> datapath accessing a stale pointer? I mean in the scenario where the
>>> process gets pre-empted between the bpf_prog_put() and
>>> rcu_assign_pointer()?
>>
>> Will call bpf_prog_put() after rcu_assign_pointer().
+1, good catch Jakub.
> I suspect you need to sync RCU or something before that.
No, see for example generic_xdp_install().
^ permalink raw reply
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
From: David Ahern @ 2017-07-28 15:10 UTC (permalink / raw)
To: Cong Wang, Roopa Prabhu; +Cc: Hangbin Liu, network dev
In-Reply-To: <CAM_iQpU5EZr1rtgY8sCBjD=utzmi9c6MQ2xb36iXLk++qSk3WA@mail.gmail.com>
On 7/27/17 10:56 PM, Cong Wang wrote:
> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> wrote:
>> On 7/26/17 12:27 PM, Roopa Prabhu wrote:
>>> agreed...so looks like the check in v3 should be
>>>
>>>
>>> + if ( rt == net->ipv6.ip6_null_entry ||
>>> + (rt->dst.error &&
>>> + #ifdef CONFIG_IPV6_MULTIPLE_TABLES
>>> + rt != net->ipv6.ip6_prohibit_entry &&
>>> + rt != net->ipv6.ip6_blk_hole_entry &&
>>> +#endif
>>> + )) {
>>> err = rt->dst.error;
>>> ip6_rt_put(rt);
>>> goto errout;
>>>
>>
>> I don't think so. If I add a prohibit route and use the fibmatch
>> attribute, I want to see the route from the FIB that was matched.
>
> But net->ipv6.ip6_prohibit_entry is not the prohibit route you can
> add in user-space, it is only used by rule actions. So do you really
> want to dump it?? My gut feeling is no, but I am definitely not sure.
>
> When you add a prohibit route, a new rt is allocated dynamically,
> net->ipv6.ip6_prohibit_entry is relatively static, internal and is the
> only one per netns. (Same for net->ipv6.ip6_blk_hole_entry)
>
> I think Hangbin's example doesn't have ip rules, so this case
> is not shown up.
>
Understood. The v4 patch returns getroute to the original behavior. The
original behavior returned a route entry not just an error code. The
following is at 5dafc87f40d7 which the commit before roopa's patch set:
# ip -6 ru add to 6000::/120 prohibit
# ip -6 ro get 6000::1
prohibit 6000::1 from :: dev lo proto kernel src 2001:db8::3 metric
4294967295 error -13 pref medium
# ip -6 ro add vrf red prohibit 5000::1/120
# ip -6 ro get vrf red 5000::1
prohibit 5000::1 from :: dev lo table red src 2001:db8::3 metric 1024
error -13 pref medium
Generically, the only time you get just an error response is when the
lookup fails to find a match and returns the null_entry which has
dst.error = -ENETUNREACH.
Now to your point about the new fibmatch option I have gone back and
forth but in the end I think returning the route associated with the FIB
rule is better than just failing with an error code.
Roopa?
^ permalink raw reply
* Re: [v3,04/10] ipw2200: constify attribute_group structures
From: Kalle Valo @ 2017-07-28 15:08 UTC (permalink / raw)
To: Arvind Yadav
Cc: davem, stas.yakovlev, sgruszka, netdev, linux-kernel,
linux-wireless
In-Reply-To: <20170728150611.96B1A60884@smtp.codeaurora.org>
Kalle Valo <kvalo@codeaurora.org> writes:
> Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
>
>> attribute_group are not supposed to change at runtime. All functions
>> working with attribute_group provided by <linux/sysfs.h> work
>> with const attribute_group. So mark the non-const structs as const.
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>
> 4 patches applied to wireless-drivers-next.git, thanks.
>
> 17830147c40a mwifiex: fix spelling mistake: "Insuffient" -> "Insufficient"
> 185ffc194800 ipw2100: don't return positive values to PCI probe on error
> af643fe9bbe0 zd1211rw: fix spelling mistake 'hybernate' -> 'hibernate'
> e66d70b789d1 brcmfmac: constify pci_device_id
Please ignore this mail, I have a bug in my patchwork script.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH net-next v11 3/4] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jamal Hadi Salim @ 2017-07-28 15:08 UTC (permalink / raw)
To: Jiri Pirko
Cc: davem, netdev, xiyou.wangcong, dsahern, eric.dumazet, mrv,
simon.horman, alex.aring
In-Reply-To: <4e835ac7-0700-726d-7f6c-6e584ff073eb@mojatatu.com>
On 17-07-28 10:52 AM, Jamal Hadi Salim wrote:
> On 17-07-28 10:12 AM, Jiri Pirko wrote:
>> Fri, Jul 28, 2017 at 03:41:44PM CEST, jhs@mojatatu.com wrote:
> [..]
>>
>> Looks like a big mess to be honest. Mixing up u32* u32 void*. I don't
>> understand ****. Would be probably good to first apply my review comment
>> on the function itselt, then to add the checks :)
>>
>
> I havent even compiled/test that Jiri.
> Just ignore the void * and assume it is a u32 *.
>
This compiled - but dont have much time right now to test.
===
static int validate_nla_bitfield32(const struct nlattr *nla,
u32 *valid_flags_allowed)
{
const struct nla_bitfield32 *bf = nla_data(nla);
u32 *valid_flags_mask = valid_flags_allowed;
if (!valid_flags_allowed)
return -EINVAL;
/*disallow invalid selector */
if ((bf->selector & *valid_flags_allowed) > *valid_flags_allowed)
return -EINVAL;
/*disallow invalid bit values */
if (bf->value & ~*valid_flags_mask)
return -EINVAL;
/*disallow valid bit values that are not selected*/
if (bf->value & ~bf->selector)
return -EINVAL;
return 0;
}
========
cheers,
jamal
^ permalink raw reply
* Re: wl3501_cs: fix spelling mistake: "Insupported" -> "Unsupported"
From: Kalle Valo @ 2017-07-28 15:07 UTC (permalink / raw)
To: Colin Ian King; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20170727220954.12105-1-colin.king@canonical.com>
Colin Ian King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in printk message
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Patch applied to wireless-drivers-next.git, thanks.
fcc870d76a2c wl3501_cs: fix spelling mistake: "Insupported" -> "Unsupported"
--
https://patchwork.kernel.org/patch/9867579/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [v3,04/10] ipw2200: constify attribute_group structures
From: Kalle Valo @ 2017-07-28 15:06 UTC (permalink / raw)
To: Arvind Yadav
Cc: davem, stas.yakovlev, sgruszka, netdev, linux-kernel,
linux-wireless
In-Reply-To: <1500371102-14758-1-git-send-email-arvind.yadav.cs@gmail.com>
Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
> attribute_group are not supposed to change at runtime. All functions
> working with attribute_group provided by <linux/sysfs.h> work
> with const attribute_group. So mark the non-const structs as const.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
4 patches applied to wireless-drivers-next.git, thanks.
17830147c40a mwifiex: fix spelling mistake: "Insuffient" -> "Insufficient"
185ffc194800 ipw2100: don't return positive values to PCI probe on error
af643fe9bbe0 zd1211rw: fix spelling mistake 'hybernate' -> 'hibernate'
e66d70b789d1 brcmfmac: constify pci_device_id
--
https://patchwork.kernel.org/patch/9847373/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH net] bpf: fix bpf_prog_get_info_by_fd to dump correct xlated_prog_len
From: Daniel Borkmann @ 2017-07-28 15:05 UTC (permalink / raw)
To: davem; +Cc: kafai, alexei.starovoitov, netdev, Daniel Borkmann
bpf_prog_size(prog->len) is not the correct length we want to dump
back to user space. The code in bpf_prog_get_info_by_fd() uses this
to copy prog->insnsi to user space, but bpf_prog_size(prog->len) also
includes the size of struct bpf_prog itself plus program instructions
and is usually used either in context of accounting or for bpf_prog_alloc()
et al, thus we copy out of bounds in bpf_prog_get_info_by_fd()
potentially. Use the correct bpf_prog_insn_size() instead.
Fixes: 1e2709769086 ("bpf: Add BPF_OBJ_GET_INFO_BY_FD")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
kernel/bpf/syscall.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 84bb399..6c772ad 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1312,7 +1312,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
}
ulen = info.xlated_prog_len;
- info.xlated_prog_len = bpf_prog_size(prog->len);
+ info.xlated_prog_len = bpf_prog_insn_size(prog);
if (info.xlated_prog_len && ulen) {
uinsns = u64_to_user_ptr(info.xlated_prog_insns);
ulen = min_t(u32, info.xlated_prog_len, ulen);
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net-next v11 1/4] net netlink: Add new type NLA_BITFIELD_32
From: Jamal Hadi Salim @ 2017-07-28 15:04 UTC (permalink / raw)
To: David Ahern, davem
Cc: netdev, jiri, xiyou.wangcong, eric.dumazet, mrv, simon.horman,
alex.aring
In-Reply-To: <886e9fe5-d523-4841-1a81-e5671447933a@gmail.com>
On 17-07-28 10:19 AM, David Ahern wrote:
> On 7/28/17 7:51 AM, Jamal Hadi Salim wrote:
>> On 17-07-25 10:41 AM, David Ahern wrote:
>>> On 7/23/17 7:35 PM, Jamal Hadi Salim wrote:
>>>> In the most basic form, the user specifies the attribute policy as:
>>>> [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data =
>>>> &myvalidflags },
>>>>
>>>> where myvalidflags is the bit mask of the flags the kernel understands.
>>>>
>>>> If the user _does not_ provide myvalidflags then the attribute will
>>>> also be rejected.
>>>
>>> No other netlink attribute has this requirement.
>>
>> This is the first one where we have to inspect content. We add things
>> when we need them - as in this case.
>
> Sure, the validation is required. My argument is that the validation
> should be done where other attributes are validated -- inline with its
> use. Nothing about this new bitfield says it must have a generic
> validation code.
>
Response below.
>>
>>> Users of the attributes
>>> are the only ones that know if a value is valid or not (e.g, attribute
>>> passing a device index) and those are always checked in line.
>>
>> It doesnt make sense that every user of the API has to repeat that
>> validation code. Same principle as someone specifying that a type is
>> u32 and have the nla validation check it. At some point we never had
>> the u32 validation code. Then it was factored out because everyone
>> repeats the same boilerplate code.
>
> Every user of an attribute that uses a device index must verify the
> device index is valid. The same code is repeated over and over.
>
> Now you are suggesting to have 1 attribute whose content is validated by
> generic infra and the rest are validated inline by the code using it. I
> believe it is wrong and going to lead to problems.
>
Kernel side checking for device ifindex must know what a device
ifindex means.
That doesnt disqualify that the generic code checks that it
is of the same size as a signed 32b, etc. That is generic
stuff that can be factored out.
In this case:
Checking for whether bits selected are in the allowed range
that the kernel understands, that the bit value are set in
the right bit position, that the bits set in the correct bit
value position are also selected in the transaction.
That is generic code (which the content validation does).
Checking what bit 5 means - that is specific per kernel user
code. Checking that when bit 5 is selected it is only useful
if bit 3 is unselected - that is specific to the kernel code
consuming those bits.
cheers,
jamal
^ permalink raw reply
* Re: [11/11] brcmfmac: constify pci_device_id
From: Kalle Valo @ 2017-07-28 15:04 UTC (permalink / raw)
To: Arvind Yadav
Cc: arend.vanspriel, hante.meuleman, franky.lin, wright.feng, netdev,
linux-kernel, linux-wireless, brcm80211-dev-list.pdl,
brcm80211-dev-list
In-Reply-To: <1500315402-3439-1-git-send-email-arvind.yadav.cs@gmail.com>
Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
> pci_device_id are not supposed to change at runtime. All functions
> working with pci_device_id provided by <linux/pci.h> work with
> const pci_device_id. So mark the non-const structs as const.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Patch applied to wireless-drivers-next.git, thanks.
e66d70b789d1 brcmfmac: constify pci_device_id
--
https://patchwork.kernel.org/patch/9845919/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 0/8] net: mvpp2: add TX interrupts support
From: Thomas Petazzoni @ 2017-07-28 15:03 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux, antoine.tenart, miquel.raynal, linux-arm-kernel,
jason, andrew, sebastian.hesselbarth, gregory.clement, nadavh,
hannah, yehuday, stefanc, mw
In-Reply-To: <20170726.133533.262039701450895012.davem@davemloft.net>
Hello,
On Wed, 26 Jul 2017 13:35:33 -0700 (PDT), David Miller wrote:
> > - This series depends on the previous series sent by Antoine Ténart
> > "net: mvpp2: MAC/GoP configuration and optional PHYs". Functionally
> > speaking there is no real dependency, but we touch in a few areas
> > the same piece of code, so I based my patch series on top of
> > Antoine's.
> >
> > - Please do not apply the last patch of this series "arm64: dts:
> > marvell: add TX interrupts for PPv2.2", it will be taken by the ARM
> > mvebu maintainers.
>
> Please don't do things this way.
>
> Patiently wait for Antione's series to make it into my tree, then
> submit your's.
I'll resubmit when Antoine's series is merged. The idea of submitting
my patches was to allow others to review/test them, not necessarily to
have them merged right away. Perhaps I should have marked them RFC to
make it clear that I don't expect you to merge them right away.
> Also, if we're continually going to elide the DTS file patches, just
It's the dutty of the sub-architecture maintainers to merge the DTS
changes. Merging them through your tree doesn't work really well, as
the changes might very likely conflict with other DTS changes merged by
the sub-architecture maintainer.
But it makes sense to have such DTS changes in the same patch series,
since they are really related. It's very common to have patch series
that contain patches that will be merged by different maintainers.
> don't bother adding them to the series. That way you don't have to
> give me special instructions, and I don't have the possibility of
> making a mistake and applying it accidently.
... but OK, I won't resend them, so you don't get potentially confused
by such patches.
Thanks!
Thomas Petazzoni
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: zd1211rw: fix spelling mistake 'hybernate' -> 'hibernate'
From: Kalle Valo @ 2017-07-28 15:02 UTC (permalink / raw)
To: Colin Ian King
Cc: Daniel Drake, Ulrich Kunitz, linux-wireless, netdev, linux-kernel
In-Reply-To: <20170713224829.15459-1-colin.king@canonical.com>
Colin Ian King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in PDEBUG debug message.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Patch applied to wireless-drivers-next.git, thanks.
af643fe9bbe0 zd1211rw: fix spelling mistake 'hybernate' -> 'hibernate'
--
https://patchwork.kernel.org/patch/9839689/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: rt2x00: make const array glrt_table static
From: Kalle Valo @ 2017-07-28 15:01 UTC (permalink / raw)
To: Colin Ian King
Cc: Stanislaw Gruszka, Helmut Schaa, linux-wireless, kernel-janitors,
netdev
In-Reply-To: <20170711114733.15387-1-colin.king@canonical.com>
Colin Ian King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Don't populate array glrt_table on the stack but make it static.
> Makes the object code a smaller by over 670 bytes:
>
> Before:
> text data bss dec hex filename
> 131772 4733 0 136505 21539 rt2800lib.o
>
> After:
> text data bss dec hex filename
> 131043 4789 0 135832 21298 rt2800lib.o
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>
Patch applied to wireless-drivers-next.git, thanks.
17830147c40a mwifiex: fix spelling mistake: "Insuffient" -> "Insufficient"
--
https://patchwork.kernel.org/patch/9834559/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH net-next v11 3/4] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jiri Pirko @ 2017-07-28 14:57 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: davem, netdev, xiyou.wangcong, dsahern, eric.dumazet, mrv,
simon.horman, alex.aring
In-Reply-To: <4e835ac7-0700-726d-7f6c-6e584ff073eb@mojatatu.com>
Fri, Jul 28, 2017 at 04:52:02PM CEST, jhs@mojatatu.com wrote:
>On 17-07-28 10:12 AM, Jiri Pirko wrote:
>> Fri, Jul 28, 2017 at 03:41:44PM CEST, jhs@mojatatu.com wrote:
>[..]
>>
>> Looks like a big mess to be honest. Mixing up u32* u32 void*. I don't
>> understand ****. Would be probably good to first apply my review comment
>> on the function itselt, then to add the checks :)
>>
>
>I havent even compiled/test that Jiri.
>Just ignore the void * and assume it is a u32 *.
>
>I am trying to avoid doing unlucky number 13 patch.
I'll rather wait for you to send the code in proper shape so I can
decypher easily and don't have to guess.
>So feedback on this is good. Just look at what it is disallowing
>first.
>
>back later.
>
>cheers,
>jamal
>
>>
>> > I can think of.
>> >
>> > static int validate_nla_bitfield32(const struct nlattr *nla,
>> > void *valid_flags_allowed)
>> > {
>> > const struct nla_bitfield32 *bf = nla_data(nla);
>> > u32 *valid_flags_mask = valid_flags_allowed;
>> >
>> > if (!valid_flags_allowed)
>> > return -EINVAL;
>> > /*disallow invalid selector */
>> > if ((bf->selector & valid_flags_allowed) >*valid_flags_allowed)
>> > return -EINVAL;
>> > /*disallow invalid bit values */
>> > if (bf->value & ~*valid_flags_mask)
>> > return -EINVAL;
>> > /*disallow valid bit values that are not selected*/
>> > if (bf->value & ~nbf->selector)
>> > return -EINVAL;
>> >
>> > return 0;
>> > }
>> >
>> > cheers,
>> > jamal
>
^ permalink raw reply
* Re: [PATCH net-next v11 1/4] net netlink: Add new type NLA_BITFIELD_32
From: Jiri Pirko @ 2017-07-28 14:55 UTC (permalink / raw)
To: David Ahern
Cc: Jamal Hadi Salim, davem, netdev, xiyou.wangcong, eric.dumazet,
mrv, simon.horman, alex.aring
In-Reply-To: <886e9fe5-d523-4841-1a81-e5671447933a@gmail.com>
Fri, Jul 28, 2017 at 04:19:06PM CEST, dsahern@gmail.com wrote:
>On 7/28/17 7:51 AM, Jamal Hadi Salim wrote:
>> On 17-07-25 10:41 AM, David Ahern wrote:
>>> On 7/23/17 7:35 PM, Jamal Hadi Salim wrote:
>>>> In the most basic form, the user specifies the attribute policy as:
>>>> [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data =
>>>> &myvalidflags },
>>>>
>>>> where myvalidflags is the bit mask of the flags the kernel understands.
>>>>
>>>> If the user _does not_ provide myvalidflags then the attribute will
>>>> also be rejected.
>>>
>>> No other netlink attribute has this requirement.
>>
>> This is the first one where we have to inspect content. We add things
>> when we need them - as in this case.
>
>Sure, the validation is required. My argument is that the validation
>should be done where other attributes are validated -- inline with its
>use. Nothing about this new bitfield says it must have a generic
>validation code.
>
>>
>>> Users of the attributes
>>> are the only ones that know if a value is valid or not (e.g, attribute
>>> passing a device index) and those are always checked in line.
>>
>> It doesnt make sense that every user of the API has to repeat that
>> validation code. Same principle as someone specifying that a type is
>> u32 and have the nla validation check it. At some point we never had
>> the u32 validation code. Then it was factored out because everyone
>> repeats the same boilerplate code.
>
>Every user of an attribute that uses a device index must verify the
>device index is valid. The same code is repeated over and over.
This is something different. You don't have NLA_IFINDEX. If you'd have it,
might make sense to do validation on Netlink level. Ofc this is highly
hypothetical. But in Jamal's case, there is indeed NLA_BITFIELD32 and
this attribute type itself assumes some format. Therefore the validation
on Netlink level makes sense here. At least that is how I feel it.
>
>Now you are suggesting to have 1 attribute whose content is validated by
>generic infra and the rest are validated inline by the code using it. I
>believe it is wrong and going to lead to problems.
^ 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