Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH iproute2 -master 1/2] bpf: update printing of generic xdp mode
From: Sergei Shtylyov @ 2017-05-13  9:01 UTC (permalink / raw)
  To: Daniel Borkmann, stephen; +Cc: alexei.starovoitov, netdev
In-Reply-To: <3c90a0b9480c61746220636ed48320da53410b6f.1494635217.git.daniel@iogearbox.net>

Hello!

On 5/13/2017 3:32 AM, Daniel Borkmann wrote:

> Follow-up to d67b9cd28c1d ("xdp: refine xdp api with regards to
> generic xdp") in order to update the XDP dumping part.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  ip/iplink_xdp.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
> index a1380ee..98503fa 100644
> --- a/ip/iplink_xdp.c
> +++ b/ip/iplink_xdp.c
> @@ -79,17 +79,20 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic)
>  void xdp_dump(FILE *fp, struct rtattr *xdp)
>  {
>  	struct rtattr *tb[IFLA_XDP_MAX + 1];
> -	__u32 flags = 0;
> +	__u8 mode;
>
>  	parse_rtattr_nested(tb, IFLA_XDP_MAX, xdp);
>
> -	if (!tb[IFLA_XDP_ATTACHED] ||
> -	    !rta_getattr_u8(tb[IFLA_XDP_ATTACHED]))
> +	if (!tb[IFLA_XDP_ATTACHED])
>  		return;
>
> -	if (tb[IFLA_XDP_FLAGS])
> -		flags = rta_getattr_u32(tb[IFLA_XDP_FLAGS]);
> -
> -	fprintf(fp, "xdp%s ",
> -		flags & XDP_FLAGS_SKB_MODE ? "generic" : "");
> +	mode = rta_getattr_u8(tb[IFLA_XDP_ATTACHED]);
> +	if (mode == XDP_ATTACHED_NONE)
> +		return;
> +	else if (mode == XDP_ATTACHED_DRV)
> +		fprintf(fp, "xdp ");
> +	else if (mode == XDP_ATTACHED_SKB)
> +		fprintf(fp, "xdpgeneric ");
> +	else
> +		fprintf(fp, "xdp[%u] ", mode);

    This is asking to be a *switch* statement.

[...]

MBR, Sergei

^ permalink raw reply

* Re: [PATCH iproute2 -master 1/2] bpf: update printing of generic xdp mode
From: Daniel Borkmann @ 2017-05-13  9:19 UTC (permalink / raw)
  To: Sergei Shtylyov, stephen; +Cc: alexei.starovoitov, netdev
In-Reply-To: <d012ef18-8689-81ef-8f56-ef2b9c673b44@cogentembedded.com>

On 05/13/2017 11:01 AM, Sergei Shtylyov wrote:
> On 5/13/2017 3:32 AM, Daniel Borkmann wrote:
>
>> Follow-up to d67b9cd28c1d ("xdp: refine xdp api with regards to
>> generic xdp") in order to update the XDP dumping part.
>>
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> ---
>>  ip/iplink_xdp.c | 19 +++++++++++--------
>>  1 file changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
>> index a1380ee..98503fa 100644
>> --- a/ip/iplink_xdp.c
>> +++ b/ip/iplink_xdp.c
>> @@ -79,17 +79,20 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic)
>>  void xdp_dump(FILE *fp, struct rtattr *xdp)
>>  {
>>      struct rtattr *tb[IFLA_XDP_MAX + 1];
>> -    __u32 flags = 0;
>> +    __u8 mode;
>>
>>      parse_rtattr_nested(tb, IFLA_XDP_MAX, xdp);
>>
>> -    if (!tb[IFLA_XDP_ATTACHED] ||
>> -        !rta_getattr_u8(tb[IFLA_XDP_ATTACHED]))
>> +    if (!tb[IFLA_XDP_ATTACHED])
>>          return;
>>
>> -    if (tb[IFLA_XDP_FLAGS])
>> -        flags = rta_getattr_u32(tb[IFLA_XDP_FLAGS]);
>> -
>> -    fprintf(fp, "xdp%s ",
>> -        flags & XDP_FLAGS_SKB_MODE ? "generic" : "");
>> +    mode = rta_getattr_u8(tb[IFLA_XDP_ATTACHED]);
>> +    if (mode == XDP_ATTACHED_NONE)
>> +        return;
>> +    else if (mode == XDP_ATTACHED_DRV)
>> +        fprintf(fp, "xdp ");
>> +    else if (mode == XDP_ATTACHED_SKB)
>> +        fprintf(fp, "xdpgeneric ");
>> +    else
>> +        fprintf(fp, "xdp[%u] ", mode);
>
>     This is asking to be a *switch* statement.

The code is fine as-is, thank you !

^ permalink raw reply

* Re: [PATCH v1] samples/bpf: Add a .gitignore for binaries
From: Mickaël Salaün @ 2017-05-13 10:30 UTC (permalink / raw)
  To: David Ahern, Alexei Starovoitov, Daniel Borkmann
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Wang Nan, netdev
In-Reply-To: <8277c7cc-8f44-40ec-1f7e-53c404d84292@cumulusnetworks.com>


[-- Attachment #1.1: Type: text/plain, Size: 849 bytes --]


On 13/02/2017 02:43, David Ahern wrote:
> On 2/12/17 2:23 PM, Mickaël Salaün wrote:
>> diff --git a/samples/bpf/.gitignore b/samples/bpf/.gitignore
>> new file mode 100644
>> index 000000000000..a7562a5ef4c2
>> --- /dev/null
>> +++ b/samples/bpf/.gitignore
>> @@ -0,0 +1,32 @@
>> +fds_example
>> +lathist
> 
> ...
> 
> Listing each target is going to be a PITA to maintain. It would be
> better to put targets into a build directory (bin?) and ignore the
> directory.
> 

It would require a lot of modifications to the Makefile and more
complexity. It seems much more simple for everyone to stick to a simple
gitignore file easily maintainable:
$ awk '$1 == "hostprogs-y" { print $3 }' < Makefile > .gitignore

Alexei, Daniel, what do you think about this? Do you want me to send a
v2 with the new tests?

 Mickaël


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* You Just Won Qatar Foundation Donation**Treat Urgently
From: QATAR, FOUNDATION @ 2017-05-13 10:52 UTC (permalink / raw)
  To: Recipients

Dear Beneficiary,

You have been selected to receive (950,000.00 EURO) as charity donations / aid from the Qatar Foundation. Please kindly provide the
below information details mention.


1. Your Full Name:
2. Country:
3. Marital Status:
4. Age:
5. Occupation:
6. Telephone:
7. Home Address:

Note: This donation funds  will be send to you Via ATM Master Card

Yours sincerely,
Engineer Saad Al Muhannadi.
President of the Qatar Foundation.

^ 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 10:54 UTC (permalink / raw)
  To: David Ahern; +Cc: David Miller, mq, netdev, roopa
In-Reply-To: <6670aa0f-2f07-5afd-7c47-35b006c9b2e7@gmail.com>

On Sat, May 13, 2017 at 12:52:47AM -0600, David Ahern wrote:
> On 5/12/17 3:41 PM, Jan Moskyto Matejka wrote:
> > On Fri, May 12, 2017 at 10:26:08AM -0700, David Ahern wrote:
> >> On 5/12/17 8:24 AM, David Miller wrote:
> >>> From: Jan Moskyto Matejka <mq@ucw.cz>
> >>> Date: Fri, 12 May 2017 13:15:10 +0200
> >>>
> >>>> -int rt6_dump_route(struct rt6_info *rt, void *p_arg);
> >>>> +int rt6_dump_route(struct rt6_info *rt, void *p_arg, int truncate);
> >>>
> >>> Please use "bool" and "true"/"false" for boolean values.
> >>>
> >>> What does ipv4 do in this situation?
> >>>
> >>> I'm hesitant to be OK with adding a new nlmsg flag just for this case
> >>> if we solve this problem differently and using existing mechanisms
> >>> elsewhere.
> >>>
> >>
> >> I'll take a look at this later today or this weekend; we can't just
> >> truncate the route returned to userspace.
> > 
> > Agreed. My favourite would be skb realloc somewhere inside the dump loop
> > ... but I don't know whether it's feasible.
> > 
> > MQ
> > 
> 
> Here is what is happening: user initiates a route dump and specifies a
> buffer size for receiving the message which becomes max_recvmsg_len.
> This buffer size dictates the skb length allocated by netlink_dump.
> 
> The dump is interrupted when a route does not fit in the skb and
> returns. Subsequent call picks up with the next route to be dumped - the
> one that overflowed. All good and normal so far.
> 
> If the next route is larger than max_recvmsg_len, then the route can not
> be put in the buffer, nothing is returned to the user which causes the
> dump to abort showing the abbreviated output. This problem occurs with
> IPv4 and IPv6. You can see this with modest size routes by just dropping
> the buffer size to something really small (e.g., with iproute2, change
> buf size in rtnl_dump_filter_l to say 2048).

Ergo no skb realloc is possible.

> 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.

> 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".

Thanks for looking into that!
MQ

^ permalink raw reply

* Re: [PATCH V2 net 1/1] smc: switch to usage of IB_PD_UNSAFE_GLOBAL_RKEY
From: Leon Romanovsky @ 2017-05-13 11:29 UTC (permalink / raw)
  To: Ursula Braun
  Cc: 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: <20170512170952.39863-2-ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 732 bytes --]

On Fri, May 12, 2017 at 07:09:52PM +0200, Ursula Braun wrote:
> This patch makes users aware of the current security implications
> when using smc.
>
> The final fix to resolve the reported security issue is worked on;
> respective patches will follow as soon as possible.
>
> Signed-off-by: Ursula Braun <ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

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 ...).

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox