Netdev List
 help / color / mirror / Atom feed
* Re: Bug report: broadcast address as incomplete entry in arp table, effectively a blackhole; reproducer included
From: Lars Ellenberg @ 2014-11-19 14:39 UTC (permalink / raw)
  To: netdev; +Cc: Olaf Kirch
In-Reply-To: <20140924161847.GG7118@soda.linbit>

Ping ...
Any ideas?

	Lars

On Wed, Sep 24, 2014 at 06:18:47PM +0200, Lars Ellenberg wrote:
> 
> You have some interface you want to broadcast on,
> so you resolve its broadcast address (once),
> and keep sending (e.g. some continuous status updates, or "heartbeat").
> 
> For some reason that interface goes down.
> If you keep sending to the previously resolved address,
> that will create an incomplete arp entry.
> 
> Unfortunately, that entry *stays* there, even if the interface is then
> brought back up (with the same network and broadcast settings).
> Once the interface is up, you won't be able to delete that entry
> (because, its a broadcast address; that arp entry is not supposed to be
> there anyways; that deletion request will be filtered out early...)
> 
> Anyone trying to send to that broadcast address will now effectively
> send to a black hole: there is an incomplete arp entry.
> 
> 
> Fix is then to stop all processes sending to that address,
> bring down the device, delete the arp entry, bring it back up,
> and then continue all processes sending to that address.
> 
> 
> I can reproduce this easily with the script below, anywhere I tested,
> on a large variety of platforms and kernels.
> (you'll obviously have to adjust DEV, BROADCAST and possibly PORT).
> 
> 
> I suspect this is not intentional, but there is simply some
> neigh_flush_dev() or similar missing "somewhere".
> 
> If you want to point me in the right direction as to probable values of
> "somewhere", I'll likely be able to figure out a minimal patch myself.
> 
> If you know the right place to fix this from the top of your head,
> even better ;-)
> 
> Thanks,
> 	Lars
> 
> ------------------------------------------------------
> 
> #!/bin/bash
> 
> DEV=eth1
> BROADCAST=192.168.133.255
> PORT=6666
> 
> send_udp_broadcast()
> {
> exec python -c '
> from socket import *
> from time import sleep
> from struct import pack
> from datetime import datetime
> 
> 
> s = socket(AF_INET, SOCK_DGRAM)
> s.setsockopt(SOL_SOCKET, SO_BROADCAST, 1)
> s.setsockopt(SOL_SOCKET, SO_BINDTODEVICE, pack("'$(( 1+${#DEV} ))'s","'$DEV'"))
> 
> while 1:
>    s.sendto(datetime.now().strftime("hi there, it is now %T.%f"), ("'$BROADCAST'",'$PORT'))
>    sleep(0.1)
> '
> }
> 
> p() { printf "\n"; printf "::: %s\n" "$@"; printf "%s\n" "-----------"; }
> 
> arp -n | grep $BROADCAST && {
> 	echo >&2 "Sorry, fix the arp table first!"
> 	exit 1
> }
> 
> ( set +x ; send_udp_broadcast ) &
> kid=$!
> p "[$kid] Started to send udp broadcasts on $DEV to $BROADCAST:$PORT"
> 
> p "We should see the packets being sent on $DEV"
> tcpdump -n -i $DEV -c 2 -xX udp and port $PORT
> 
> p "We should not have any arp entries for $BROADCAST"
> arp -n | grep $BROADCAST
> 
> p "But we soon will, after we take down $DEV"
> ip link set down $DEV
> 
> sleep 2
> 
> p "Now we should have an incomplete arp entry for $BROADCAST"
> arp -n | grep $BROADCAST
> 
> p "It will still be there after we bring $DEV back up"
> ip link set up $DEV
> 
> p "There won't be any packets now, this should timeout:"
> while read -r -t 5 line ; do
> 	echo "$line"
> done < <(tcpdump -n -i $DEV -c 2 -xX udp and port $PORT 2>&1)
> 
> p "Because we still have that arp entry"
> arp -n | grep $BROADCAST
> 
> p "And we cannot get rid of it, either, as long as this $DEV is up"
> arp -d $BROADCAST
> arp -n | grep $BROADCAST
> 
> p "but we can stop the child," \
>   "take down the device," \
>   "remove the arp entry then," \
>   "and bring the device back up"
> 
> kill -STOP $kid
> ip link set down $DEV
> sleep 1
> arp -d $BROADCAST
> ip link set up $DEV
> arp -n | grep $BROADCAST
> 
> p "continue the child" \
>   "and now we should see outgoing packets again"
> 
> kill -CONT $kid
> tcpdump -n -i $DEV -c 2 -xX udp and port $PORT
> 
> p "and no more arp entry"
> arp -n | grep $BROADCAST
> 
> p "Done."
> kill $kid

^ permalink raw reply

* Re: [PATCH v3] fix locking regression in ipx_sendmsg and ipx_recvmsg
From: Sergei Shtylyov @ 2014-11-19 14:38 UTC (permalink / raw)
  To: Jiri Bohac, Arnd Bergmann; +Cc: Arnaldo Carvalho de Melo, netdev, David Miller
In-Reply-To: <20141119103814.GB19092@midget.suse.cz>

Hello.

On 11/19/2014 1:38 PM, Jiri Bohac wrote:

> This fixes an old regression introduced by commit
> b0d0d915 (ipx: remove the BKL).

> When a recvmsg syscall blocks waiting for new data, no data can be sent on the
> same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.

> This breaks mars-nwe (NetWare emulator):
> - the ncpserv process reads the request using recvmsg
> - ncpserv forks and spawns nwconn
> - ncpserv calls a (blocking) recvmsg and waits for new requests
> - nwconn deadlocks in sendmsg on the same socket

> Commit b0d0d915 has simply replaced BKL locking with
> lock_sock/release_sock. Unlike now, BKL got unlocked while
> sleeping, so a blocking recvmsg did not block a concurrent
> sendmsg.

> Only keep the socket locked while actually working with the socket data and
> release it prior to calling skb_recv_datagram().


> Signed-off-by: Jiri Bohac <jbohac@suse.cz>

> diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
> index a0c7536..d0725d9 100644
> --- a/net/ipx/af_ipx.c
> +++ b/net/ipx/af_ipx.c
> @@ -1764,6 +1764,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
>   	struct ipxhdr *ipx = NULL;
>   	struct sk_buff *skb;
>   	int copied, rc;
> +	int locked = 1;

    Why not *bool*?

[...]

WBR, Sergei

^ permalink raw reply

* lånetilbud
From: CENTRAL TRUST LOAN LIMITED @ 2014-11-19 14:24 UTC (permalink / raw)




Vi er et selskap med økonomisk bistand. Vi låne penger ut til individer trenger økonomisk hjelp, som har en dårlig kreditt posten eller behov for penger til å betale regninger, å investere på forretningsreise. Jeg vil bruke dette mediet til å informere deg om at vi gjengi pålitelig Betalingsmottakers hjelp som jeg vil gjerne tilby deg et lån på 3% så ikke nøl med å søke om et lån fordi vi skal gi deg best av vår tjeneste.

SØKERINFORMASJON

* Navn på søkeren:
* Adressen til søkeren:
* By:
* Land:
* Kjønn:
* Sivilstand:
* Alder:
* Månedlig inntekt:
* Yrke:
* Tel :/ Mobil:
* Beløp:
* Lån varighet:
* E-post:

Du kan kontakte oss via vår e-postadresse: (centraltrust_ltd@webadicta.org)
TEL: +447053866981
Vi venter på ditt svar.
David Donald

-- 
Esta mensagem foi verificada pelo sistema de antivirus e
 acredita-se estar livre de perigo.

^ permalink raw reply

* Re: [PATCH net-next v2 1/2] r8152: adjust r8152_submit_rx
From: Sergei Shtylyov @ 2014-11-19 14:02 UTC (permalink / raw)
  To: Hayes Wang, netdev; +Cc: nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-96-Taiwan-albertk@realtek.com>

Hello.

On 11/19/2014 8:20 AM, Hayes Wang wrote:

> The behavior of handling the returned status from r8152_submit_rx()
> is almost same, so let r8152_submit_rx() deal with the error
> directly. This could avoid the duplicate code.

> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
>   drivers/net/usb/r8152.c | 41 +++++++++++++++++++++--------------------
>   1 file changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 0a30fd3..df0868b 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
[...]
> @@ -1806,11 +1789,29 @@ static void bottom_half(unsigned long data)
>   static
>   int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags)
>   {
> +	int ret = 0;

    Initialization not needed.

> +
>   	usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
>   			  agg->head, agg_buf_sz,
>   			  (usb_complete_t)read_bulk_callback, agg);
>
> -	return usb_submit_urb(agg->urb, mem_flags);
> +	ret = usb_submit_urb(agg->urb, mem_flags);
> +

    Empty line not needed here either.

> +	if (ret == -ENODEV) {
> +		set_bit(RTL8152_UNPLUG, &tp->flags);
> +		netif_device_detach(tp->netdev);
> +	} else if (ret) {
> +		struct urb *urb = agg->urb;
> +		unsigned long flags;
> +
> +		urb->actual_length = 0;
> +		spin_lock_irqsave(&tp->rx_lock, flags);
> +		list_add_tail(&agg->list, &tp->rx_done);
> +		spin_unlock_irqrestore(&tp->rx_lock, flags);
> +		tasklet_schedule(&tp->tl);
> +	}
> +
> +	return ret;
>   }
[...]

WBR, Sergei

^ permalink raw reply

* Re: [patch net-next v2 02/10] net: introduce generic switch devices support
From: Roopa Prabhu @ 2014-11-19 13:59 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
	f.fainelli, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <20141119134645.GE1926@nanopsycho.orion>

On 11/19/14, 5:46 AM, Jiri Pirko wrote:
> Wed, Nov 19, 2014 at 02:28:10PM CET, roopa@cumulusnetworks.com wrote:
>> On 11/9/14, 2:51 AM, Jiri Pirko wrote:
>>> The goal of this is to provide a possibility to support various switch
>>> chips. Drivers should implement relevant ndos to do so. Now there is
>>> only one ndo defined:
>>> - for getting physical switch id is in place.
>>>
>>> Note that user can use random port netdevice to access the switch.
>>>
>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>> ---
>>>   Documentation/networking/switchdev.txt | 59 ++++++++++++++++++++++++++++++++++
>>>   MAINTAINERS                            |  7 ++++
>>>   include/linux/netdevice.h              | 10 ++++++
>>>   include/net/switchdev.h                | 30 +++++++++++++++++
>>>   net/Kconfig                            |  1 +
>>>   net/Makefile                           |  3 ++
>>>   net/switchdev/Kconfig                  | 13 ++++++++
>>>   net/switchdev/Makefile                 |  5 +++
>>>   net/switchdev/switchdev.c              | 33 +++++++++++++++++++
>>>   9 files changed, 161 insertions(+)
>>>   create mode 100644 Documentation/networking/switchdev.txt
>>>   create mode 100644 include/net/switchdev.h
>>>   create mode 100644 net/switchdev/Kconfig
>>>   create mode 100644 net/switchdev/Makefile
>>>   create mode 100644 net/switchdev/switchdev.c
>>>
>>> diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
>>> new file mode 100644
>>> index 0000000..98be76c
>>> --- /dev/null
>>> +++ b/Documentation/networking/switchdev.txt
>>> @@ -0,0 +1,59 @@
>>> +Switch (and switch-ish) device drivers HOWTO
>>> +===========================
>>> +
>>> +Please note that the word "switch" is here used in very generic meaning.
>>> +This include devices supporting L2/L3 but also various flow offloading chips,
>>> +including switches embedded into SR-IOV NICs.
>>> +
>>> +Lets describe a topology a bit. Imagine the following example:
>>> +
>>> +       +----------------------------+    +---------------+
>>> +       |     SOME switch chip       |    |      CPU      |
>>> +       +----------------------------+    +---------------+
>>> +       port1 port2 port3 port4 MNGMNT    |     PCI-E     |
>>> +         |     |     |     |     |       +---------------+
>>> +        PHY   PHY    |     |     |         |  NIC0 NIC1
>>> +                     |     |     |         |   |    |
>>> +                     |     |     +- PCI-E -+   |    |
>>> +                     |     +------- MII -------+    |
>>> +                     +------------- MII ------------+
>>> +
>>> +In this example, there are two independent lines between the switch silicon
>>> +and CPU. NIC0 and NIC1 drivers are not aware of a switch presence. They are
>>> +separate from the switch driver. SOME switch chip is by managed by a driver
>>> +via PCI-E device MNGMNT. Note that MNGMNT device, NIC0 and NIC1 may be
>>> +connected to some other type of bus.
>>> +
>>> +Now, for the previous example show the representation in kernel:
>>> +
>>> +       +----------------------------+    +---------------+
>>> +       |     SOME switch chip       |    |      CPU      |
>>> +       +----------------------------+    +---------------+
>>> +       sw0p0 sw0p1 sw0p2 sw0p3 MNGMNT    |     PCI-E     |
>>> +         |     |     |     |     |       +---------------+
>>> +        PHY   PHY    |     |     |         |  eth0 eth1
>>> +                     |     |     |         |   |    |
>>> +                     |     |     +- PCI-E -+   |    |
>>> +                     |     +------- MII -------+    |
>>> +                     +------------- MII ------------+
>>> +
>>> +Lets call the example switch driver for SOME switch chip "SOMEswitch". This
>>> +driver takes care of PCI-E device MNGMNT. There is a netdevice instance sw0pX
>>> +created for each port of a switch. These netdevices are instances
>>> +of "SOMEswitch" driver. sw0pX netdevices serve as a "representation"
>>> +of the switch chip. eth0 and eth1 are instances of some other existing driver.
>>> +
>>> +The only difference of the switch-port netdevice from the ordinary netdevice
>>> +is that is implements couple more NDOs:
>>> +
>>> +	ndo_sw_parent_get_id - This returns the same ID for two port netdevices
>>> +			       of the same physical switch chip. This is
>>> +			       mandatory to be implemented by all switch drivers
>>> +			       and serves the caller for recognition of a port
>>> +			       netdevice.
>>> +	ndo_sw_parent_* - Functions that serve for a manipulation of the switch
>>> +			  chip itself (it can be though of as a "parent" of the
>>> +			  port, therefore the name). They are not port-specific.
>>> +			  Caller might use arbitrary port netdevice of the same
>>> +			  switch and it will make no difference.
>>> +	ndo_sw_port_* - Functions that serve for a port-specific manipulation.
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 3a41fb0..776e078 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -9003,6 +9003,13 @@ F:	lib/swiotlb.c
>>>   F:	arch/*/kernel/pci-swiotlb.c
>>>   F:	include/linux/swiotlb.h
>>> +SWITCHDEV
>>> +M:	Jiri Pirko <jiri@resnulli.us>
>>> +L:	netdev@vger.kernel.org
>>> +S:	Supported
>>> +F:	net/switchdev/
>>> +F:	include/net/switchdev.h
>>> +
>>>   SYNOPSYS ARC ARCHITECTURE
>>>   M:	Vineet Gupta <vgupta@synopsys.com>
>>>   S:	Supported
>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>> index 71922e0..97eade9 100644
>>> --- a/include/linux/netdevice.h
>>> +++ b/include/linux/netdevice.h
>>> @@ -1017,6 +1017,12 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>>>    *	performing GSO on a packet. The device returns true if it is
>>>    *	able to GSO the packet, false otherwise. If the return value is
>>>    *	false the stack will do software GSO.
>>> + *
>>> + * int (*ndo_sw_parent_id_get)(struct net_device *dev,
>>> + *			       struct netdev_phys_item_id *psid);
>>> + *	Called to get an ID of the switch chip this port is part of.
>>> + *	If driver implements this, it indicates that it represents a port
>>> + *	of a switch chip.
>>>    */
>>>   struct net_device_ops {
>>>   	int			(*ndo_init)(struct net_device *dev);
>>> @@ -1168,6 +1174,10 @@ struct net_device_ops {
>>>   	int			(*ndo_get_lock_subclass)(struct net_device *dev);
>>>   	bool			(*ndo_gso_check) (struct sk_buff *skb,
>>>   						  struct net_device *dev);
>>> +#ifdef CONFIG_NET_SWITCHDEV
>>> +	int			(*ndo_sw_parent_id_get)(struct net_device *dev,
>>> +							struct netdev_phys_item_id *psid);
>> Can we keep the name generic and not include "sw" which implies switch here
>> ?.
>> I understand that it is under CONFIG_NET_SWITCHDEV but we might find use for
>> them in other offload scenarios in the future.
>> This particular ndo can be just ndo_parent_id_get().
>> And the others that do specific offloads can have "offload" in them if
>> required..?.
>
> But this is for getting parent switch id, sw should be there.

Since we have not figured out the details or namespace for switchd ids 
yet, its still some parent id to me.

> If comes a
> time when this might be reused to something else, we change it then.
> This is internal api, easily changeable.
also, "sw" seems more "software"  than "switch".

>
>>
>>
>>> +#endif
>>>   };
>>>   /**
>>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>> new file mode 100644
>>> index 0000000..79bf9bd
>>> --- /dev/null
>>> +++ b/include/net/switchdev.h
>>> @@ -0,0 +1,30 @@
>>> +/*
>>> + * include/net/switchdev.h - Switch device API
>>> + * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
>>> + *
>>> + * This program is free software; you can redistribute 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.
>>> + */
>>> +#ifndef _LINUX_SWITCHDEV_H_
>>> +#define _LINUX_SWITCHDEV_H_
>>> +
>>> +#include <linux/netdevice.h>
>>> +
>>> +#ifdef CONFIG_NET_SWITCHDEV
>>> +
>>> +int netdev_sw_parent_id_get(struct net_device *dev,
>>> +			    struct netdev_phys_item_id *psid);
>>> +
>>> +#else
>>> +
>>> +static inline int netdev_sw_parent_id_get(struct net_device *dev,
>>> +					  struct netdev_phys_item_id *psid)
>>> +{
>>> +	return -EOPNOTSUPP;
>>> +}
>>> +
>>> +#endif
>>> +
>>> +#endif /* _LINUX_SWITCHDEV_H_ */
>>> diff --git a/net/Kconfig b/net/Kconfig
>>> index 99815b5..ff9ffc1 100644
>>> --- a/net/Kconfig
>>> +++ b/net/Kconfig
>>> @@ -228,6 +228,7 @@ source "net/vmw_vsock/Kconfig"
>>>   source "net/netlink/Kconfig"
>>>   source "net/mpls/Kconfig"
>>>   source "net/hsr/Kconfig"
>>> +source "net/switchdev/Kconfig"
>>>   config RPS
>>>   	boolean
>>> diff --git a/net/Makefile b/net/Makefile
>>> index 7ed1970..95fc694 100644
>>> --- a/net/Makefile
>>> +++ b/net/Makefile
>>> @@ -73,3 +73,6 @@ obj-$(CONFIG_OPENVSWITCH)	+= openvswitch/
>>>   obj-$(CONFIG_VSOCKETS)	+= vmw_vsock/
>>>   obj-$(CONFIG_NET_MPLS_GSO)	+= mpls/
>>>   obj-$(CONFIG_HSR)		+= hsr/
>>> +ifneq ($(CONFIG_NET_SWITCHDEV),)
>>> +obj-y				+= switchdev/
>>> +endif
>>> diff --git a/net/switchdev/Kconfig b/net/switchdev/Kconfig
>>> new file mode 100644
>>> index 0000000..1557545
>>> --- /dev/null
>>> +++ b/net/switchdev/Kconfig
>>> @@ -0,0 +1,13 @@
>>> +#
>>> +# Configuration for Switch device support
>>> +#
>>> +
>>> +config NET_SWITCHDEV
>>> +	boolean "Switch (and switch-ish) device support (EXPERIMENTAL)"
>>> +	depends on INET
>>> +	---help---
>>> +	  This module provides glue between core networking code and device
>>> +	  drivers in order to support hardware switch chips in very generic
>>> +	  meaning of the word "switch". This include devices supporting L2/L3 but
>>> +	  also various flow offloading chips, including switches embedded into
>>> +	  SR-IOV NICs.
>>> diff --git a/net/switchdev/Makefile b/net/switchdev/Makefile
>>> new file mode 100644
>>> index 0000000..5ed63ed
>>> --- /dev/null
>>> +++ b/net/switchdev/Makefile
>>> @@ -0,0 +1,5 @@
>>> +#
>>> +# Makefile for the Switch device API
>>> +#
>>> +
>>> +obj-$(CONFIG_NET_SWITCHDEV) += switchdev.o
>>> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>> new file mode 100644
>>> index 0000000..5010f646
>>> --- /dev/null
>>> +++ b/net/switchdev/switchdev.c
>>> @@ -0,0 +1,33 @@
>>> +/*
>>> + * net/switchdev/switchdev.c - Switch device API
>>> + * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
>>> + *
>>> + * This program is free software; you can redistribute 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.
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/types.h>
>>> +#include <linux/init.h>
>>> +#include <linux/netdevice.h>
>>> +#include <net/switchdev.h>
>>> +
>>> +/**
>>> + *	netdev_sw_parent_id_get - Get ID of a switch
>>> + *	@dev: port device
>>> + *	@psid: switch ID
>>> + *
>>> + *	Get ID of a switch this port is part of.
>>> + */
>>> +int netdev_sw_parent_id_get(struct net_device *dev,
>>> +			    struct netdev_phys_item_id *psid)
>>> +{
>>> +	const struct net_device_ops *ops = dev->netdev_ops;
>>> +
>>> +	if (!ops->ndo_sw_parent_id_get)
>>> +		return -EOPNOTSUPP;
>>> +	return ops->ndo_sw_parent_id_get(dev, psid);
>>> +}
>>> +EXPORT_SYMBOL(netdev_sw_parent_id_get);
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: BCM4313 & brcmsmac & 3.12: only semi-working?
From: Arend van Spriel @ 2014-11-19 13:49 UTC (permalink / raw)
  To: Maximilian Engelhardt
  Cc: Rafał Miłecki, Michael Tokarev, Seth Forshee,
	brcm80211 development,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Network Development
In-Reply-To: <546C9F2B.8090104-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On 11/19/14 14:46, Arend van Spriel wrote:
> On 11/17/14 23:36, Maximilian Engelhardt wrote:
>> On Thursday 09 October 2014 10:21:12 Rafał Miłecki wrote:
>>> On 9 October 2014 09:52, Arend van Spriel<arend-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>> I have been staring at differences brcmsmac and our internal driver
>>>> (which
>>>> wl is derived from as well). Would you be able to make an mmiotrace
>>>> loading
>>>> wl and brcmsmac.
>>>
>>> Arend,
>>>
>>> As you noticed earlier, this is a WiFi card with BT. I've checked
>>> siutils.c from SDK 7.14.43.7 (got it from Netgear R8000) and it has
>>> three BCM4313 workarounds:
>>>
>>>
>>> 1) si_doattach
>>> SI_MSG(("Applying 4313 WARs\n"));
>>> si_pmu_chipcontrol(sih, 0, CCTRL_4313_12MA_LED_DRIVE,
>>> CCTRL_4313_12MA_LED_DRIVE);
>>>
>>> It's already implemented in bcma, see driver_chipcommon_pmu.c
>>>
>>>
>>> 2) si_epa_4313war
>>> /* EPA Fix */
>>> W_REG(sii->osh,&cc->gpiocontrol,
>>> R_REG(sii->osh,&cc->gpiocontrol) | GPIO_CTRL_EPA_EN_MASK);
>>>
>>> This is what you already implement in ai_epa_4313war
>>>
>>>
>>> 3) si_pmu_synth_pwrsw_4313_war
>>> I don't think it's implemented.
>>>
>>>
>>> 4) si_btcombo_p250_4313_war
>>> I also don't think it's implemented
>>>
>>>
>>> The last one (si_btcombo_p250_4313_war) looks promising, the comment
>>> above the function says:
>>> /** WL/BT control for 4313 btcombo boards>= P250 */
>>> I guess you need to re-implement this function and call it somewhere
>>> in brcmsmac.
>>
>> Hi,
>>
>> I just wanted to ask if there is any progress on this issue since I
>> haven't
>> heard anything for a month. Please let me know if you need any additional
>> information.
>
> I had a bit of problem with the mmiotrace files. Well, my (virtual)
> linux machine had as it ended up swapping on my disk and went down the
> drain.
>
> I still wanted to look at items 3) and 4) that Rafal mentioned although
> item 4) does not seems to apply to Michael's 4313 it may help yours. I
> submitted a patch to get counter statistics from the 802.11 core on the
> chip [1] as I am curious whether that will show a possible lead.
>
> Regards,
> Arend

+ links

patch [1] depends on patch [2]

[1] 
https://git.kernel.org/cgit/linux/kernel/git/linville/wireless-next.git/commit/drivers/net/wireless/brcm80211?id=3fe33c4ce
[2] 
https://git.kernel.org/cgit/linux/kernel/git/linville/wireless-next.git/commit/drivers/net/wireless/brcm80211?id=9146782b1b
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 net-next v2 02/10] net: introduce generic switch devices support
From: Jiri Pirko @ 2014-11-19 13:46 UTC (permalink / raw)
  To: Roopa Prabhu
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
	f.fainelli, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <546C9AEA.2020209@cumulusnetworks.com>

Wed, Nov 19, 2014 at 02:28:10PM CET, roopa@cumulusnetworks.com wrote:
>On 11/9/14, 2:51 AM, Jiri Pirko wrote:
>>The goal of this is to provide a possibility to support various switch
>>chips. Drivers should implement relevant ndos to do so. Now there is
>>only one ndo defined:
>>- for getting physical switch id is in place.
>>
>>Note that user can use random port netdevice to access the switch.
>>
>>Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>---
>>  Documentation/networking/switchdev.txt | 59 ++++++++++++++++++++++++++++++++++
>>  MAINTAINERS                            |  7 ++++
>>  include/linux/netdevice.h              | 10 ++++++
>>  include/net/switchdev.h                | 30 +++++++++++++++++
>>  net/Kconfig                            |  1 +
>>  net/Makefile                           |  3 ++
>>  net/switchdev/Kconfig                  | 13 ++++++++
>>  net/switchdev/Makefile                 |  5 +++
>>  net/switchdev/switchdev.c              | 33 +++++++++++++++++++
>>  9 files changed, 161 insertions(+)
>>  create mode 100644 Documentation/networking/switchdev.txt
>>  create mode 100644 include/net/switchdev.h
>>  create mode 100644 net/switchdev/Kconfig
>>  create mode 100644 net/switchdev/Makefile
>>  create mode 100644 net/switchdev/switchdev.c
>>
>>diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
>>new file mode 100644
>>index 0000000..98be76c
>>--- /dev/null
>>+++ b/Documentation/networking/switchdev.txt
>>@@ -0,0 +1,59 @@
>>+Switch (and switch-ish) device drivers HOWTO
>>+===========================
>>+
>>+Please note that the word "switch" is here used in very generic meaning.
>>+This include devices supporting L2/L3 but also various flow offloading chips,
>>+including switches embedded into SR-IOV NICs.
>>+
>>+Lets describe a topology a bit. Imagine the following example:
>>+
>>+       +----------------------------+    +---------------+
>>+       |     SOME switch chip       |    |      CPU      |
>>+       +----------------------------+    +---------------+
>>+       port1 port2 port3 port4 MNGMNT    |     PCI-E     |
>>+         |     |     |     |     |       +---------------+
>>+        PHY   PHY    |     |     |         |  NIC0 NIC1
>>+                     |     |     |         |   |    |
>>+                     |     |     +- PCI-E -+   |    |
>>+                     |     +------- MII -------+    |
>>+                     +------------- MII ------------+
>>+
>>+In this example, there are two independent lines between the switch silicon
>>+and CPU. NIC0 and NIC1 drivers are not aware of a switch presence. They are
>>+separate from the switch driver. SOME switch chip is by managed by a driver
>>+via PCI-E device MNGMNT. Note that MNGMNT device, NIC0 and NIC1 may be
>>+connected to some other type of bus.
>>+
>>+Now, for the previous example show the representation in kernel:
>>+
>>+       +----------------------------+    +---------------+
>>+       |     SOME switch chip       |    |      CPU      |
>>+       +----------------------------+    +---------------+
>>+       sw0p0 sw0p1 sw0p2 sw0p3 MNGMNT    |     PCI-E     |
>>+         |     |     |     |     |       +---------------+
>>+        PHY   PHY    |     |     |         |  eth0 eth1
>>+                     |     |     |         |   |    |
>>+                     |     |     +- PCI-E -+   |    |
>>+                     |     +------- MII -------+    |
>>+                     +------------- MII ------------+
>>+
>>+Lets call the example switch driver for SOME switch chip "SOMEswitch". This
>>+driver takes care of PCI-E device MNGMNT. There is a netdevice instance sw0pX
>>+created for each port of a switch. These netdevices are instances
>>+of "SOMEswitch" driver. sw0pX netdevices serve as a "representation"
>>+of the switch chip. eth0 and eth1 are instances of some other existing driver.
>>+
>>+The only difference of the switch-port netdevice from the ordinary netdevice
>>+is that is implements couple more NDOs:
>>+
>>+	ndo_sw_parent_get_id - This returns the same ID for two port netdevices
>>+			       of the same physical switch chip. This is
>>+			       mandatory to be implemented by all switch drivers
>>+			       and serves the caller for recognition of a port
>>+			       netdevice.
>>+	ndo_sw_parent_* - Functions that serve for a manipulation of the switch
>>+			  chip itself (it can be though of as a "parent" of the
>>+			  port, therefore the name). They are not port-specific.
>>+			  Caller might use arbitrary port netdevice of the same
>>+			  switch and it will make no difference.
>>+	ndo_sw_port_* - Functions that serve for a port-specific manipulation.
>>diff --git a/MAINTAINERS b/MAINTAINERS
>>index 3a41fb0..776e078 100644
>>--- a/MAINTAINERS
>>+++ b/MAINTAINERS
>>@@ -9003,6 +9003,13 @@ F:	lib/swiotlb.c
>>  F:	arch/*/kernel/pci-swiotlb.c
>>  F:	include/linux/swiotlb.h
>>+SWITCHDEV
>>+M:	Jiri Pirko <jiri@resnulli.us>
>>+L:	netdev@vger.kernel.org
>>+S:	Supported
>>+F:	net/switchdev/
>>+F:	include/net/switchdev.h
>>+
>>  SYNOPSYS ARC ARCHITECTURE
>>  M:	Vineet Gupta <vgupta@synopsys.com>
>>  S:	Supported
>>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>index 71922e0..97eade9 100644
>>--- a/include/linux/netdevice.h
>>+++ b/include/linux/netdevice.h
>>@@ -1017,6 +1017,12 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>>   *	performing GSO on a packet. The device returns true if it is
>>   *	able to GSO the packet, false otherwise. If the return value is
>>   *	false the stack will do software GSO.
>>+ *
>>+ * int (*ndo_sw_parent_id_get)(struct net_device *dev,
>>+ *			       struct netdev_phys_item_id *psid);
>>+ *	Called to get an ID of the switch chip this port is part of.
>>+ *	If driver implements this, it indicates that it represents a port
>>+ *	of a switch chip.
>>   */
>>  struct net_device_ops {
>>  	int			(*ndo_init)(struct net_device *dev);
>>@@ -1168,6 +1174,10 @@ struct net_device_ops {
>>  	int			(*ndo_get_lock_subclass)(struct net_device *dev);
>>  	bool			(*ndo_gso_check) (struct sk_buff *skb,
>>  						  struct net_device *dev);
>>+#ifdef CONFIG_NET_SWITCHDEV
>>+	int			(*ndo_sw_parent_id_get)(struct net_device *dev,
>>+							struct netdev_phys_item_id *psid);
>Can we keep the name generic and not include "sw" which implies switch here
>?.
>I understand that it is under CONFIG_NET_SWITCHDEV but we might find use for
>them in other offload scenarios in the future.
>This particular ndo can be just ndo_parent_id_get().
>And the others that do specific offloads can have "offload" in them if
>required..?.


But this is for getting parent switch id, sw should be there. If comes a
time when this might be reused to something else, we change it then.
This is internal api, easily changeable.

>
>
>
>>+#endif
>>  };
>>  /**
>>diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>new file mode 100644
>>index 0000000..79bf9bd
>>--- /dev/null
>>+++ b/include/net/switchdev.h
>>@@ -0,0 +1,30 @@
>>+/*
>>+ * include/net/switchdev.h - Switch device API
>>+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
>>+ *
>>+ * This program is free software; you can redistribute 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.
>>+ */
>>+#ifndef _LINUX_SWITCHDEV_H_
>>+#define _LINUX_SWITCHDEV_H_
>>+
>>+#include <linux/netdevice.h>
>>+
>>+#ifdef CONFIG_NET_SWITCHDEV
>>+
>>+int netdev_sw_parent_id_get(struct net_device *dev,
>>+			    struct netdev_phys_item_id *psid);
>>+
>>+#else
>>+
>>+static inline int netdev_sw_parent_id_get(struct net_device *dev,
>>+					  struct netdev_phys_item_id *psid)
>>+{
>>+	return -EOPNOTSUPP;
>>+}
>>+
>>+#endif
>>+
>>+#endif /* _LINUX_SWITCHDEV_H_ */
>>diff --git a/net/Kconfig b/net/Kconfig
>>index 99815b5..ff9ffc1 100644
>>--- a/net/Kconfig
>>+++ b/net/Kconfig
>>@@ -228,6 +228,7 @@ source "net/vmw_vsock/Kconfig"
>>  source "net/netlink/Kconfig"
>>  source "net/mpls/Kconfig"
>>  source "net/hsr/Kconfig"
>>+source "net/switchdev/Kconfig"
>>  config RPS
>>  	boolean
>>diff --git a/net/Makefile b/net/Makefile
>>index 7ed1970..95fc694 100644
>>--- a/net/Makefile
>>+++ b/net/Makefile
>>@@ -73,3 +73,6 @@ obj-$(CONFIG_OPENVSWITCH)	+= openvswitch/
>>  obj-$(CONFIG_VSOCKETS)	+= vmw_vsock/
>>  obj-$(CONFIG_NET_MPLS_GSO)	+= mpls/
>>  obj-$(CONFIG_HSR)		+= hsr/
>>+ifneq ($(CONFIG_NET_SWITCHDEV),)
>>+obj-y				+= switchdev/
>>+endif
>>diff --git a/net/switchdev/Kconfig b/net/switchdev/Kconfig
>>new file mode 100644
>>index 0000000..1557545
>>--- /dev/null
>>+++ b/net/switchdev/Kconfig
>>@@ -0,0 +1,13 @@
>>+#
>>+# Configuration for Switch device support
>>+#
>>+
>>+config NET_SWITCHDEV
>>+	boolean "Switch (and switch-ish) device support (EXPERIMENTAL)"
>>+	depends on INET
>>+	---help---
>>+	  This module provides glue between core networking code and device
>>+	  drivers in order to support hardware switch chips in very generic
>>+	  meaning of the word "switch". This include devices supporting L2/L3 but
>>+	  also various flow offloading chips, including switches embedded into
>>+	  SR-IOV NICs.
>>diff --git a/net/switchdev/Makefile b/net/switchdev/Makefile
>>new file mode 100644
>>index 0000000..5ed63ed
>>--- /dev/null
>>+++ b/net/switchdev/Makefile
>>@@ -0,0 +1,5 @@
>>+#
>>+# Makefile for the Switch device API
>>+#
>>+
>>+obj-$(CONFIG_NET_SWITCHDEV) += switchdev.o
>>diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
>>new file mode 100644
>>index 0000000..5010f646
>>--- /dev/null
>>+++ b/net/switchdev/switchdev.c
>>@@ -0,0 +1,33 @@
>>+/*
>>+ * net/switchdev/switchdev.c - Switch device API
>>+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
>>+ *
>>+ * This program is free software; you can redistribute 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.
>>+ */
>>+
>>+#include <linux/kernel.h>
>>+#include <linux/types.h>
>>+#include <linux/init.h>
>>+#include <linux/netdevice.h>
>>+#include <net/switchdev.h>
>>+
>>+/**
>>+ *	netdev_sw_parent_id_get - Get ID of a switch
>>+ *	@dev: port device
>>+ *	@psid: switch ID
>>+ *
>>+ *	Get ID of a switch this port is part of.
>>+ */
>>+int netdev_sw_parent_id_get(struct net_device *dev,
>>+			    struct netdev_phys_item_id *psid)
>>+{
>>+	const struct net_device_ops *ops = dev->netdev_ops;
>>+
>>+	if (!ops->ndo_sw_parent_id_get)
>>+		return -EOPNOTSUPP;
>>+	return ops->ndo_sw_parent_id_get(dev, psid);
>>+}
>>+EXPORT_SYMBOL(netdev_sw_parent_id_get);
>

^ permalink raw reply

* Re: BCM4313 & brcmsmac & 3.12: only semi-working?
From: Arend van Spriel @ 2014-11-19 13:46 UTC (permalink / raw)
  To: Maximilian Engelhardt
  Cc: Rafał Miłecki, Michael Tokarev, Seth Forshee,
	brcm80211 development, linux-wireless@vger.kernel.org,
	Network Development
In-Reply-To: <3543341.FmUQFH9nrl@eisbaer>

On 11/17/14 23:36, Maximilian Engelhardt wrote:
> On Thursday 09 October 2014 10:21:12 Rafał Miłecki wrote:
>> On 9 October 2014 09:52, Arend van Spriel<arend@broadcom.com>  wrote:
>>> I have been staring at differences brcmsmac and our internal driver (which
>>> wl is derived from as well). Would you be able to make an mmiotrace
>>> loading
>>> wl and brcmsmac.
>>
>> Arend,
>>
>> As you noticed earlier, this is a WiFi card with BT. I've checked
>> siutils.c from SDK 7.14.43.7 (got it from Netgear R8000) and it has
>> three BCM4313 workarounds:
>>
>>
>> 1) si_doattach
>> SI_MSG(("Applying 4313 WARs\n"));
>> si_pmu_chipcontrol(sih, 0, CCTRL_4313_12MA_LED_DRIVE,
>> CCTRL_4313_12MA_LED_DRIVE);
>>
>> It's already implemented in bcma, see driver_chipcommon_pmu.c
>>
>>
>> 2) si_epa_4313war
>> /* EPA Fix */
>> W_REG(sii->osh,&cc->gpiocontrol,
>> R_REG(sii->osh,&cc->gpiocontrol) | GPIO_CTRL_EPA_EN_MASK);
>>
>> This is what you already implement in ai_epa_4313war
>>
>>
>> 3) si_pmu_synth_pwrsw_4313_war
>> I don't think it's implemented.
>>
>>
>> 4) si_btcombo_p250_4313_war
>> I also don't think it's implemented
>>
>>
>> The last one (si_btcombo_p250_4313_war) looks promising, the comment
>> above the function says:
>> /** WL/BT control for 4313 btcombo boards>= P250 */
>> I guess you need to re-implement this function and call it somewhere
>> in brcmsmac.
>
> Hi,
>
> I just wanted to ask if there is any progress on this issue since I haven't
> heard anything for a month. Please let me know if you need any additional
> information.

I had a bit of problem with the mmiotrace files. Well, my (virtual) 
linux machine had as it ended up swapping on my disk and went down the 
drain.

I still wanted to look at items 3) and 4) that Rafal mentioned although 
item 4) does not seems to apply to Michael's 4313 it may help yours. I 
submitted a patch to get counter statistics from the 802.11 core on the 
chip [1] as I am curious whether that will show a possible lead.

Regards,
Arend

^ permalink raw reply

* Re: [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls
From: Pablo Neira Ayuso @ 2014-11-19 13:40 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: David S. Miller, Jozsef Kadlecsik, Julian Anastasov,
	Patrick McHardy, Simon Horman, Wensong Zhang, netdev, lvs-devel,
	netfilter-devel, coreteam, LKML, kernel-janitors, Coccinelle
In-Reply-To: <546BA253.6030100@users.sourceforge.net>

On Tue, Nov 18, 2014 at 08:47:31PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 18 Nov 2014 20:37:05 +0100
> 
> The functions free_percpu() and module_put() test whether their argument
> is NULL and then return immediately. Thus the test around the call is
> not needed.

@IPVS folks: Since this involves a nf_tables specific chunk, ack your
chunks and I'll put this into nf-next to speed up things.

Thanks.

> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  net/netfilter/ipvs/ip_vs_ctl.c   | 3 +--
>  net/netfilter/ipvs/ip_vs_pe.c    | 3 +--
>  net/netfilter/ipvs/ip_vs_sched.c | 3 +--
>  net/netfilter/ipvs/ip_vs_sync.c  | 3 +--
>  net/netfilter/nf_tables_api.c    | 3 +--
>  5 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index fd3f444..7c5e40a 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -465,8 +465,7 @@ __ip_vs_bind_svc(struct ip_vs_dest *dest, struct ip_vs_service *svc)
>  
>  static void ip_vs_service_free(struct ip_vs_service *svc)
>  {
> -	if (svc->stats.cpustats)
> -		free_percpu(svc->stats.cpustats);
> +	free_percpu(svc->stats.cpustats);
>  	kfree(svc);
>  }
>  
> diff --git a/net/netfilter/ipvs/ip_vs_pe.c b/net/netfilter/ipvs/ip_vs_pe.c
> index 1a82b29..0df17ca 100644
> --- a/net/netfilter/ipvs/ip_vs_pe.c
> +++ b/net/netfilter/ipvs/ip_vs_pe.c
> @@ -37,8 +37,7 @@ struct ip_vs_pe *__ip_vs_pe_getbyname(const char *pe_name)
>  			rcu_read_unlock();
>  			return pe;
>  		}
> -		if (pe->module)
> -			module_put(pe->module);
> +		module_put(pe->module);
>  	}
>  	rcu_read_unlock();
>  
> diff --git a/net/netfilter/ipvs/ip_vs_sched.c b/net/netfilter/ipvs/ip_vs_sched.c
> index 4dbcda6..199760c 100644
> --- a/net/netfilter/ipvs/ip_vs_sched.c
> +++ b/net/netfilter/ipvs/ip_vs_sched.c
> @@ -104,8 +104,7 @@ static struct ip_vs_scheduler *ip_vs_sched_getbyname(const char *sched_name)
>  			mutex_unlock(&ip_vs_sched_mutex);
>  			return sched;
>  		}
> -		if (sched->module)
> -			module_put(sched->module);
> +		module_put(sched->module);
>  	}
>  
>  	mutex_unlock(&ip_vs_sched_mutex);
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index eadffb2..cafe28d 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -820,8 +820,7 @@ ip_vs_conn_fill_param_sync(struct net *net, int af, union ip_vs_sync_conn *sc,
>  
>  		p->pe_data = kmemdup(pe_data, pe_data_len, GFP_ATOMIC);
>  		if (!p->pe_data) {
> -			if (p->pe->module)
> -				module_put(p->pe->module);
> +			module_put(p->pe->module);
>  			return -ENOMEM;
>  		}
>  		p->pe_data_len = pe_data_len;
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index deeb95f..b115f54 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -3472,8 +3472,7 @@ static int nf_tables_abort(struct sk_buff *skb)
>  			break;
>  		case NFT_MSG_NEWCHAIN:
>  			if (nft_trans_chain_update(trans)) {
> -				if (nft_trans_chain_stats(trans))
> -					free_percpu(nft_trans_chain_stats(trans));
> +				free_percpu(nft_trans_chain_stats(trans));
>  
>  				nft_trans_destroy(trans);
>  			} else {
> -- 
> 2.1.3
> 

^ permalink raw reply

* Re: [patch net-next v2 02/10] net: introduce generic switch devices support
From: Roopa Prabhu @ 2014-11-19 13:28 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
	pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
	xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
	f.fainelli, linville, jasowang, ebiederm, nicolas.dichtel,
	ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
	Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
	mleitner, shrijeet, gospo, bcrl
In-Reply-To: <1415530280-9190-3-git-send-email-jiri@resnulli.us>

On 11/9/14, 2:51 AM, Jiri Pirko wrote:
> The goal of this is to provide a possibility to support various switch
> chips. Drivers should implement relevant ndos to do so. Now there is
> only one ndo defined:
> - for getting physical switch id is in place.
>
> Note that user can use random port netdevice to access the switch.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>   Documentation/networking/switchdev.txt | 59 ++++++++++++++++++++++++++++++++++
>   MAINTAINERS                            |  7 ++++
>   include/linux/netdevice.h              | 10 ++++++
>   include/net/switchdev.h                | 30 +++++++++++++++++
>   net/Kconfig                            |  1 +
>   net/Makefile                           |  3 ++
>   net/switchdev/Kconfig                  | 13 ++++++++
>   net/switchdev/Makefile                 |  5 +++
>   net/switchdev/switchdev.c              | 33 +++++++++++++++++++
>   9 files changed, 161 insertions(+)
>   create mode 100644 Documentation/networking/switchdev.txt
>   create mode 100644 include/net/switchdev.h
>   create mode 100644 net/switchdev/Kconfig
>   create mode 100644 net/switchdev/Makefile
>   create mode 100644 net/switchdev/switchdev.c
>
> diff --git a/Documentation/networking/switchdev.txt b/Documentation/networking/switchdev.txt
> new file mode 100644
> index 0000000..98be76c
> --- /dev/null
> +++ b/Documentation/networking/switchdev.txt
> @@ -0,0 +1,59 @@
> +Switch (and switch-ish) device drivers HOWTO
> +===========================
> +
> +Please note that the word "switch" is here used in very generic meaning.
> +This include devices supporting L2/L3 but also various flow offloading chips,
> +including switches embedded into SR-IOV NICs.
> +
> +Lets describe a topology a bit. Imagine the following example:
> +
> +       +----------------------------+    +---------------+
> +       |     SOME switch chip       |    |      CPU      |
> +       +----------------------------+    +---------------+
> +       port1 port2 port3 port4 MNGMNT    |     PCI-E     |
> +         |     |     |     |     |       +---------------+
> +        PHY   PHY    |     |     |         |  NIC0 NIC1
> +                     |     |     |         |   |    |
> +                     |     |     +- PCI-E -+   |    |
> +                     |     +------- MII -------+    |
> +                     +------------- MII ------------+
> +
> +In this example, there are two independent lines between the switch silicon
> +and CPU. NIC0 and NIC1 drivers are not aware of a switch presence. They are
> +separate from the switch driver. SOME switch chip is by managed by a driver
> +via PCI-E device MNGMNT. Note that MNGMNT device, NIC0 and NIC1 may be
> +connected to some other type of bus.
> +
> +Now, for the previous example show the representation in kernel:
> +
> +       +----------------------------+    +---------------+
> +       |     SOME switch chip       |    |      CPU      |
> +       +----------------------------+    +---------------+
> +       sw0p0 sw0p1 sw0p2 sw0p3 MNGMNT    |     PCI-E     |
> +         |     |     |     |     |       +---------------+
> +        PHY   PHY    |     |     |         |  eth0 eth1
> +                     |     |     |         |   |    |
> +                     |     |     +- PCI-E -+   |    |
> +                     |     +------- MII -------+    |
> +                     +------------- MII ------------+
> +
> +Lets call the example switch driver for SOME switch chip "SOMEswitch". This
> +driver takes care of PCI-E device MNGMNT. There is a netdevice instance sw0pX
> +created for each port of a switch. These netdevices are instances
> +of "SOMEswitch" driver. sw0pX netdevices serve as a "representation"
> +of the switch chip. eth0 and eth1 are instances of some other existing driver.
> +
> +The only difference of the switch-port netdevice from the ordinary netdevice
> +is that is implements couple more NDOs:
> +
> +	ndo_sw_parent_get_id - This returns the same ID for two port netdevices
> +			       of the same physical switch chip. This is
> +			       mandatory to be implemented by all switch drivers
> +			       and serves the caller for recognition of a port
> +			       netdevice.
> +	ndo_sw_parent_* - Functions that serve for a manipulation of the switch
> +			  chip itself (it can be though of as a "parent" of the
> +			  port, therefore the name). They are not port-specific.
> +			  Caller might use arbitrary port netdevice of the same
> +			  switch and it will make no difference.
> +	ndo_sw_port_* - Functions that serve for a port-specific manipulation.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3a41fb0..776e078 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9003,6 +9003,13 @@ F:	lib/swiotlb.c
>   F:	arch/*/kernel/pci-swiotlb.c
>   F:	include/linux/swiotlb.h
>   
> +SWITCHDEV
> +M:	Jiri Pirko <jiri@resnulli.us>
> +L:	netdev@vger.kernel.org
> +S:	Supported
> +F:	net/switchdev/
> +F:	include/net/switchdev.h
> +
>   SYNOPSYS ARC ARCHITECTURE
>   M:	Vineet Gupta <vgupta@synopsys.com>
>   S:	Supported
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 71922e0..97eade9 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1017,6 +1017,12 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>    *	performing GSO on a packet. The device returns true if it is
>    *	able to GSO the packet, false otherwise. If the return value is
>    *	false the stack will do software GSO.
> + *
> + * int (*ndo_sw_parent_id_get)(struct net_device *dev,
> + *			       struct netdev_phys_item_id *psid);
> + *	Called to get an ID of the switch chip this port is part of.
> + *	If driver implements this, it indicates that it represents a port
> + *	of a switch chip.
>    */
>   struct net_device_ops {
>   	int			(*ndo_init)(struct net_device *dev);
> @@ -1168,6 +1174,10 @@ struct net_device_ops {
>   	int			(*ndo_get_lock_subclass)(struct net_device *dev);
>   	bool			(*ndo_gso_check) (struct sk_buff *skb,
>   						  struct net_device *dev);
> +#ifdef CONFIG_NET_SWITCHDEV
> +	int			(*ndo_sw_parent_id_get)(struct net_device *dev,
> +							struct netdev_phys_item_id *psid);
Can we keep the name generic and not include "sw" which implies switch 
here ?.
I understand that it is under CONFIG_NET_SWITCHDEV but we might find use 
for them in other offload scenarios in the future.
This particular ndo can be just ndo_parent_id_get().
And the others that do specific offloads can have "offload" in them if 
required..?.



> +#endif
>   };
>   
>   /**
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> new file mode 100644
> index 0000000..79bf9bd
> --- /dev/null
> +++ b/include/net/switchdev.h
> @@ -0,0 +1,30 @@
> +/*
> + * include/net/switchdev.h - Switch device API
> + * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
> + *
> + * This program is free software; you can redistribute 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.
> + */
> +#ifndef _LINUX_SWITCHDEV_H_
> +#define _LINUX_SWITCHDEV_H_
> +
> +#include <linux/netdevice.h>
> +
> +#ifdef CONFIG_NET_SWITCHDEV
> +
> +int netdev_sw_parent_id_get(struct net_device *dev,
> +			    struct netdev_phys_item_id *psid);
> +
> +#else
> +
> +static inline int netdev_sw_parent_id_get(struct net_device *dev,
> +					  struct netdev_phys_item_id *psid)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +#endif
> +
> +#endif /* _LINUX_SWITCHDEV_H_ */
> diff --git a/net/Kconfig b/net/Kconfig
> index 99815b5..ff9ffc1 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -228,6 +228,7 @@ source "net/vmw_vsock/Kconfig"
>   source "net/netlink/Kconfig"
>   source "net/mpls/Kconfig"
>   source "net/hsr/Kconfig"
> +source "net/switchdev/Kconfig"
>   
>   config RPS
>   	boolean
> diff --git a/net/Makefile b/net/Makefile
> index 7ed1970..95fc694 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -73,3 +73,6 @@ obj-$(CONFIG_OPENVSWITCH)	+= openvswitch/
>   obj-$(CONFIG_VSOCKETS)	+= vmw_vsock/
>   obj-$(CONFIG_NET_MPLS_GSO)	+= mpls/
>   obj-$(CONFIG_HSR)		+= hsr/
> +ifneq ($(CONFIG_NET_SWITCHDEV),)
> +obj-y				+= switchdev/
> +endif
> diff --git a/net/switchdev/Kconfig b/net/switchdev/Kconfig
> new file mode 100644
> index 0000000..1557545
> --- /dev/null
> +++ b/net/switchdev/Kconfig
> @@ -0,0 +1,13 @@
> +#
> +# Configuration for Switch device support
> +#
> +
> +config NET_SWITCHDEV
> +	boolean "Switch (and switch-ish) device support (EXPERIMENTAL)"
> +	depends on INET
> +	---help---
> +	  This module provides glue between core networking code and device
> +	  drivers in order to support hardware switch chips in very generic
> +	  meaning of the word "switch". This include devices supporting L2/L3 but
> +	  also various flow offloading chips, including switches embedded into
> +	  SR-IOV NICs.
> diff --git a/net/switchdev/Makefile b/net/switchdev/Makefile
> new file mode 100644
> index 0000000..5ed63ed
> --- /dev/null
> +++ b/net/switchdev/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for the Switch device API
> +#
> +
> +obj-$(CONFIG_NET_SWITCHDEV) += switchdev.o
> diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
> new file mode 100644
> index 0000000..5010f646
> --- /dev/null
> +++ b/net/switchdev/switchdev.c
> @@ -0,0 +1,33 @@
> +/*
> + * net/switchdev/switchdev.c - Switch device API
> + * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
> + *
> + * This program is free software; you can redistribute 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/init.h>
> +#include <linux/netdevice.h>
> +#include <net/switchdev.h>
> +
> +/**
> + *	netdev_sw_parent_id_get - Get ID of a switch
> + *	@dev: port device
> + *	@psid: switch ID
> + *
> + *	Get ID of a switch this port is part of.
> + */
> +int netdev_sw_parent_id_get(struct net_device *dev,
> +			    struct netdev_phys_item_id *psid)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +
> +	if (!ops->ndo_sw_parent_id_get)
> +		return -EOPNOTSUPP;
> +	return ops->ndo_sw_parent_id_get(dev, psid);
> +}
> +EXPORT_SYMBOL(netdev_sw_parent_id_get);

^ permalink raw reply

* [patch iproute2 v4] tc: add support for vlan tc action
From: Jiri Pirko @ 2014-11-19 13:08 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Reviewed-by: Cong Wang <cwang@twopensource.com>
---

v1->v2:
- included changes suggested by Jamal

 include/linux/tc_act/tc_vlan.h |  35 +++++++
 tc/Makefile                    |   1 +
 tc/m_vlan.c                    | 221 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 257 insertions(+)
 create mode 100644 include/linux/tc_act/tc_vlan.h
 create mode 100644 tc/m_vlan.c

diff --git a/include/linux/tc_act/tc_vlan.h b/include/linux/tc_act/tc_vlan.h
new file mode 100644
index 0000000..f7b8d44
--- /dev/null
+++ b/include/linux/tc_act/tc_vlan.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
+ *
+ * This program is free software; you can redistribute 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.
+ */
+
+#ifndef __LINUX_TC_VLAN_H
+#define __LINUX_TC_VLAN_H
+
+#include <linux/pkt_cls.h>
+
+#define TCA_ACT_VLAN 12
+
+#define TCA_VLAN_ACT_POP	1
+#define TCA_VLAN_ACT_PUSH	2
+
+struct tc_vlan {
+	tc_gen;
+	int v_action;
+};
+
+enum {
+	TCA_VLAN_UNSPEC,
+	TCA_VLAN_TM,
+	TCA_VLAN_PARMS,
+	TCA_VLAN_PUSH_VLAN_ID,
+	TCA_VLAN_PUSH_VLAN_PROTOCOL,
+	__TCA_VLAN_MAX,
+};
+#define TCA_VLAN_MAX (__TCA_VLAN_MAX - 1)
+
+#endif
diff --git a/tc/Makefile b/tc/Makefile
index 1ab36c6..830c97d 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -40,6 +40,7 @@ TCMODULES += m_pedit.o
 TCMODULES += m_skbedit.o
 TCMODULES += m_csum.o
 TCMODULES += m_simple.o
+TCMODULES += m_vlan.o
 TCMODULES += p_ip.o
 TCMODULES += p_icmp.o
 TCMODULES += p_tcp.o
diff --git a/tc/m_vlan.c b/tc/m_vlan.c
new file mode 100644
index 0000000..872bf72
--- /dev/null
+++ b/tc/m_vlan.c
@@ -0,0 +1,221 @@
+/*
+ * m_vlan.c		vlan manipulation module
+ *
+ *              This program is free software; you can redistribute 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:     Jiri Pirko <jiri@resnulli.us>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <linux/if_ether.h>
+#include "utils.h"
+#include "rt_names.h"
+#include "tc_util.h"
+#include <linux/tc_act/tc_vlan.h>
+
+static void explain(void)
+{
+	fprintf(stderr, "Usage: vlan pop\n");
+	fprintf(stderr, "       vlan push [ protocol VLANPROTO ] id VLANID\n");
+	fprintf(stderr, "       VLANPROTO is one of 802.1Q or 802.1AD\n");
+	fprintf(stderr, "            with default: 802.1Q\n");
+}
+
+static void usage(void)
+{
+	explain();
+	exit(-1);
+}
+
+static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
+		      int tca_id, struct nlmsghdr *n)
+{
+	int argc = *argc_p;
+	char **argv = *argv_p;
+	struct rtattr *tail;
+	int action = 0;
+	__u16 id;
+	int id_set = 0;
+	__u16 proto;
+	int proto_set = 0;
+	struct tc_vlan parm = { 0 };
+
+	if (matches(*argv, "vlan") != 0)
+		return -1;
+
+	NEXT_ARG();
+
+	while (argc > 0) {
+		if (matches(*argv, "pop") == 0) {
+			if (action) {
+				fprintf(stderr, "unexpexted \"%s\" - action already specified\n",
+					*argv);
+				explain();
+				return -1;
+			}
+			action = TCA_VLAN_ACT_POP;
+		} else if (matches(*argv, "push") == 0) {
+			if (action) {
+				fprintf(stderr, "unexpexted \"%s\" - action already specified\n",
+					*argv);
+				explain();
+				return -1;
+			}
+			action = TCA_VLAN_ACT_PUSH;
+		} else if (matches(*argv, "id") == 0) {
+			if (action != TCA_VLAN_ACT_PUSH) {
+				fprintf(stderr, "\"%s\" is only valid for push\n",
+					*argv);
+				explain();
+				return -1;
+			}
+			NEXT_ARG();
+			if (get_u16(&id, *argv, 0))
+				invarg("id is invalid", *argv);
+			id_set = 1;
+		} else if (matches(*argv, "protocol") == 0) {
+			if (action != TCA_VLAN_ACT_PUSH) {
+				fprintf(stderr, "\"%s\" is only valid for push\n",
+					*argv);
+				explain();
+				return -1;
+			}
+			NEXT_ARG();
+			if (ll_proto_a2n(&proto, *argv))
+				invarg("protocol is invalid", *argv);
+			proto_set = 1;
+		} else if (matches(*argv, "help") == 0) {
+			usage();
+		} else {
+			break;
+		}
+		argc--;
+		argv++;
+	}
+
+	parm.action = TC_ACT_PIPE;
+	if (argc) {
+		if (matches(*argv, "reclassify") == 0) {
+			parm.action = TC_ACT_RECLASSIFY;
+			NEXT_ARG();
+		} else if (matches(*argv, "pipe") == 0) {
+			parm.action = TC_ACT_PIPE;
+			NEXT_ARG();
+		} else if (matches(*argv, "drop") == 0 ||
+			   matches(*argv, "shot") == 0) {
+			parm.action = TC_ACT_SHOT;
+			NEXT_ARG();
+		} else if (matches(*argv, "continue") == 0) {
+			parm.action = TC_ACT_UNSPEC;
+			NEXT_ARG();
+		} else if (matches(*argv, "pass") == 0) {
+			parm.action = TC_ACT_OK;
+			NEXT_ARG();
+		}
+	}
+
+	if (argc) {
+		if (matches(*argv, "index") == 0) {
+			NEXT_ARG();
+			if (get_u32(&parm.index, *argv, 10)) {
+				fprintf(stderr, "vlan: Illegal \"index\"\n");
+				return -1;
+			}
+			argc--;
+			argv++;
+		}
+	}
+
+	if (action == TCA_VLAN_ACT_PUSH && !id_set) {
+		fprintf(stderr, "id needs to be set for push\n");
+		explain();
+		return -1;
+	}
+
+	parm.v_action = action;
+	tail = NLMSG_TAIL(n);
+	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
+	addattr_l(n, MAX_MSG, TCA_VLAN_PARMS, &parm, sizeof(parm));
+	if (id_set)
+		addattr_l(n, MAX_MSG, TCA_VLAN_PUSH_VLAN_ID, &id, 2);
+	if (proto_set) {
+		if (proto != htons(ETH_P_8021Q) &&
+		    proto != htons(ETH_P_8021AD)) {
+			fprintf(stderr, "protocol not supported\n");
+			explain();
+			return -1;
+		}
+
+		addattr_l(n, MAX_MSG, TCA_VLAN_PUSH_VLAN_PROTOCOL, &proto, 2);
+	}
+	tail->rta_len = (char *)NLMSG_TAIL(n) - (char *)tail;
+
+	*argc_p = argc;
+	*argv_p = argv;
+	return 0;
+}
+
+static int print_vlan(struct action_util *au, FILE *f, struct rtattr *arg)
+{
+	SPRINT_BUF(b1);
+	struct rtattr *tb[TCA_VLAN_MAX + 1];
+	__u16 val;
+	struct tc_vlan *parm;
+
+	if (arg == NULL)
+		return -1;
+
+	parse_rtattr_nested(tb, TCA_VLAN_MAX, arg);
+
+	if (!tb[TCA_VLAN_PARMS]) {
+		fprintf(f, "[NULL vlan parameters]");
+		return -1;
+	}
+	parm = RTA_DATA(tb[TCA_VLAN_PARMS]);
+
+	fprintf(f, " vlan");
+
+	switch(parm->v_action) {
+	case TCA_VLAN_ACT_POP:
+		fprintf(f, " pop");
+		break;
+	case TCA_VLAN_ACT_PUSH:
+		fprintf(f, " push");
+		if (tb[TCA_VLAN_PUSH_VLAN_ID]) {
+			val = rta_getattr_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
+			fprintf(f, " id %u", val);
+		}
+		if (tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]) {
+			fprintf(f, " protocol %s",
+				ll_proto_n2a(rta_getattr_u16(tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]),
+					     b1, sizeof(b1)));
+		}
+		break;
+	}
+
+	fprintf(f, "\n\t index %d ref %d bind %d", parm->index, parm->refcnt,
+		parm->bindcnt);
+
+	if (show_stats) {
+		if (tb[TCA_VLAN_TM]) {
+			struct tcf_t *tm = RTA_DATA(tb[TCA_VLAN_TM]);
+			print_tm(f, tm);
+		}
+	}
+
+	fprintf(f, "\n ");
+
+	return 0;
+}
+
+struct action_util vlan_action_util = {
+	.id = "vlan",
+	.parse_aopt = parse_vlan,
+	.print_aopt = print_vlan,
+};
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v4 9/9] sched: introduce vlan action
From: Jiri Pirko @ 2014-11-19 13:05 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

This tc action allows to work with vlan tagged skbs. Two supported
sub-actions are header pop and header push.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---

v1->v2:
- fixed protocol dumping in tcf_vlan_dump

 include/net/tc_act/tc_vlan.h        |  27 +++++
 include/uapi/linux/tc_act/tc_vlan.h |  35 ++++++
 net/sched/Kconfig                   |  11 ++
 net/sched/Makefile                  |   1 +
 net/sched/act_vlan.c                | 207 ++++++++++++++++++++++++++++++++++++
 5 files changed, 281 insertions(+)
 create mode 100644 include/net/tc_act/tc_vlan.h
 create mode 100644 include/uapi/linux/tc_act/tc_vlan.h
 create mode 100644 net/sched/act_vlan.c

diff --git a/include/net/tc_act/tc_vlan.h b/include/net/tc_act/tc_vlan.h
new file mode 100644
index 0000000..c809c1d
--- /dev/null
+++ b/include/net/tc_act/tc_vlan.h
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
+ *
+ * This program is free software; you can redistribute 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.
+ */
+
+#ifndef __NET_TC_VLAN_H
+#define __NET_TC_VLAN_H
+
+#include <net/act_api.h>
+
+#define VLAN_F_POP		0x1
+#define VLAN_F_PUSH		0x2
+
+struct tcf_vlan {
+	struct tcf_common	common;
+	int			tcfv_action;
+	__be16			tcfv_push_vid;
+	__be16			tcfv_push_proto;
+};
+#define to_vlan(a) \
+	container_of(a->priv, struct tcf_vlan, common)
+
+#endif /* __NET_TC_VLAN_H */
diff --git a/include/uapi/linux/tc_act/tc_vlan.h b/include/uapi/linux/tc_act/tc_vlan.h
new file mode 100644
index 0000000..f7b8d44
--- /dev/null
+++ b/include/uapi/linux/tc_act/tc_vlan.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
+ *
+ * This program is free software; you can redistribute 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.
+ */
+
+#ifndef __LINUX_TC_VLAN_H
+#define __LINUX_TC_VLAN_H
+
+#include <linux/pkt_cls.h>
+
+#define TCA_ACT_VLAN 12
+
+#define TCA_VLAN_ACT_POP	1
+#define TCA_VLAN_ACT_PUSH	2
+
+struct tc_vlan {
+	tc_gen;
+	int v_action;
+};
+
+enum {
+	TCA_VLAN_UNSPEC,
+	TCA_VLAN_TM,
+	TCA_VLAN_PARMS,
+	TCA_VLAN_PUSH_VLAN_ID,
+	TCA_VLAN_PUSH_VLAN_PROTOCOL,
+	__TCA_VLAN_MAX,
+};
+#define TCA_VLAN_MAX (__TCA_VLAN_MAX - 1)
+
+#endif
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index a1a8e29..88618f8 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -686,6 +686,17 @@ config NET_ACT_CSUM
 	  To compile this code as a module, choose M here: the
 	  module will be called act_csum.
 
+config NET_ACT_VLAN
+        tristate "Vlan manipulation"
+        depends on NET_CLS_ACT
+        ---help---
+	  Say Y here to push or pop vlan headers.
+
+	  If unsure, say N.
+
+	  To compile this code as a module, choose M here: the
+	  module will be called act_vlan.
+
 config NET_CLS_IND
 	bool "Incoming device classification"
 	depends on NET_CLS_U32 || NET_CLS_FW
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 0a869a1..679f24a 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_NET_ACT_PEDIT)	+= act_pedit.o
 obj-$(CONFIG_NET_ACT_SIMP)	+= act_simple.o
 obj-$(CONFIG_NET_ACT_SKBEDIT)	+= act_skbedit.o
 obj-$(CONFIG_NET_ACT_CSUM)	+= act_csum.o
+obj-$(CONFIG_NET_ACT_VLAN)	+= act_vlan.o
 obj-$(CONFIG_NET_SCH_FIFO)	+= sch_fifo.o
 obj-$(CONFIG_NET_SCH_CBQ)	+= sch_cbq.o
 obj-$(CONFIG_NET_SCH_HTB)	+= sch_htb.o
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
new file mode 100644
index 0000000..d735ecf
--- /dev/null
+++ b/net/sched/act_vlan.c
@@ -0,0 +1,207 @@
+/*
+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
+ *
+ * This program is free software; you can redistribute 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/skbuff.h>
+#include <linux/rtnetlink.h>
+#include <linux/if_vlan.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+
+#include <linux/tc_act/tc_vlan.h>
+#include <net/tc_act/tc_vlan.h>
+
+#define VLAN_TAB_MASK     15
+
+static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
+		    struct tcf_result *res)
+{
+	struct tcf_vlan *v = a->priv;
+	int action;
+	int err;
+
+	spin_lock(&v->tcf_lock);
+	v->tcf_tm.lastuse = jiffies;
+	bstats_update(&v->tcf_bstats, skb);
+	action = v->tcf_action;
+
+	switch (v->tcfv_action) {
+	case TCA_VLAN_ACT_POP:
+		err = skb_vlan_pop(skb);
+		if (err)
+			goto drop;
+		break;
+	case TCA_VLAN_ACT_PUSH:
+		err = skb_vlan_push(skb, v->tcfv_push_proto, v->tcfv_push_vid);
+		if (err)
+			goto drop;
+		break;
+	default:
+		BUG();
+	}
+
+	goto unlock;
+
+drop:
+	action = TC_ACT_SHOT;
+	v->tcf_qstats.drops++;
+unlock:
+	spin_unlock(&v->tcf_lock);
+	return action;
+}
+
+static const struct nla_policy vlan_policy[TCA_VLAN_MAX + 1] = {
+	[TCA_VLAN_PARMS]		= { .len = sizeof(struct tc_vlan) },
+	[TCA_VLAN_PUSH_VLAN_ID]		= { .type = NLA_U16 },
+	[TCA_VLAN_PUSH_VLAN_PROTOCOL]	= { .type = NLA_U16 },
+};
+
+static int tcf_vlan_init(struct net *net, struct nlattr *nla,
+			 struct nlattr *est, struct tc_action *a,
+			 int ovr, int bind)
+{
+	struct nlattr *tb[TCA_VLAN_MAX + 1];
+	struct tc_vlan *parm;
+	struct tcf_vlan *v;
+	int action;
+	__be16 push_vid = 0;
+	__be16 push_proto = 0;
+	int ret = 0;
+	int err;
+
+	if (!nla)
+		return -EINVAL;
+
+	err = nla_parse_nested(tb, TCA_VLAN_MAX, nla, vlan_policy);
+	if (err < 0)
+		return err;
+
+	if (!tb[TCA_VLAN_PARMS])
+		return -EINVAL;
+	parm = nla_data(tb[TCA_VLAN_PARMS]);
+	switch (parm->v_action) {
+	case TCA_VLAN_ACT_POP:
+		break;
+	case TCA_VLAN_ACT_PUSH:
+		if (!tb[TCA_VLAN_PUSH_VLAN_ID])
+			return -EINVAL;
+		push_vid = nla_get_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
+		if (push_vid >= VLAN_VID_MASK)
+			return -ERANGE;
+
+		if (tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]) {
+			push_proto = nla_get_be16(tb[TCA_VLAN_PUSH_VLAN_PROTOCOL]);
+			switch (push_proto) {
+			case htons(ETH_P_8021Q):
+			case htons(ETH_P_8021AD):
+				break;
+			default:
+				return -EPROTONOSUPPORT;
+			}
+		} else {
+			push_proto = htons(ETH_P_8021Q);
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+	action = parm->v_action;
+
+	if (!tcf_hash_check(parm->index, a, bind)) {
+		ret = tcf_hash_create(parm->index, est, a, sizeof(*v), bind);
+		if (ret)
+			return ret;
+
+		ret = ACT_P_CREATED;
+	} else {
+		if (bind)
+			return 0;
+		tcf_hash_release(a, bind);
+		if (!ovr)
+			return -EEXIST;
+	}
+
+	v = to_vlan(a);
+
+	spin_lock_bh(&v->tcf_lock);
+
+	v->tcfv_action = action;
+	v->tcfv_push_vid = push_vid;
+	v->tcfv_push_proto = push_proto;
+
+	v->tcf_action = parm->action;
+
+	spin_unlock_bh(&v->tcf_lock);
+
+	if (ret == ACT_P_CREATED)
+		tcf_hash_insert(a);
+	return ret;
+}
+
+static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
+			 int bind, int ref)
+{
+	unsigned char *b = skb_tail_pointer(skb);
+	struct tcf_vlan *v = a->priv;
+	struct tc_vlan opt = {
+		.index    = v->tcf_index,
+		.refcnt   = v->tcf_refcnt - ref,
+		.bindcnt  = v->tcf_bindcnt - bind,
+		.action   = v->tcf_action,
+		.v_action = v->tcfv_action,
+	};
+	struct tcf_t t;
+
+	if (nla_put(skb, TCA_VLAN_PARMS, sizeof(opt), &opt))
+		goto nla_put_failure;
+
+	if (v->tcfv_action == TCA_VLAN_ACT_PUSH &&
+	    (nla_put_u16(skb, TCA_VLAN_PUSH_VLAN_ID, v->tcfv_push_vid) ||
+	     nla_put_be16(skb, TCA_VLAN_PUSH_VLAN_PROTOCOL, v->tcfv_push_proto)))
+		goto nla_put_failure;
+
+	t.install = jiffies_to_clock_t(jiffies - v->tcf_tm.install);
+	t.lastuse = jiffies_to_clock_t(jiffies - v->tcf_tm.lastuse);
+	t.expires = jiffies_to_clock_t(v->tcf_tm.expires);
+	if (nla_put(skb, TCA_VLAN_TM, sizeof(t), &t))
+		goto nla_put_failure;
+	return skb->len;
+
+nla_put_failure:
+	nlmsg_trim(skb, b);
+	return -1;
+}
+
+static struct tc_action_ops act_vlan_ops = {
+	.kind		=	"vlan",
+	.type		=	TCA_ACT_VLAN,
+	.owner		=	THIS_MODULE,
+	.act		=	tcf_vlan,
+	.dump		=	tcf_vlan_dump,
+	.init		=	tcf_vlan_init,
+};
+
+static int __init vlan_init_module(void)
+{
+	return tcf_register_action(&act_vlan_ops, VLAN_TAB_MASK);
+}
+
+static void __exit vlan_cleanup_module(void)
+{
+	tcf_unregister_action(&act_vlan_ops);
+}
+
+module_init(vlan_init_module);
+module_exit(vlan_cleanup_module);
+
+MODULE_AUTHOR("Jiri Pirko <jiri@resnulli.us>");
+MODULE_DESCRIPTION("vlan manipulation actions");
+MODULE_LICENSE("GPL v2");
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v4 8/9] net: move vlan pop/push functions into common code
From: Jiri Pirko @ 2014-11-19 13:05 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

So it can be used from out of openvswitch code.
Did couple of cosmetic changes on the way, namely variable naming and
adding support for 8021AD proto.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---

v3->v4:
- don't try to be smart and copy skb->mac_len manipulation as it is. It can be
  adjusted later on.
- fix error path in do_execute_actions as suggested by Pravin
v2->v3:
- used previously introduced __vlan_insert_tag helper
- used skb_push/pull to get skb->data into needed point
- fixed skb->mac_len computation in skb_vlan_push pointed out by Pravin
v1->v2:
- adjusted to fix recent ovs changes
- included change to use make_writable suggested by Eric

 include/linux/skbuff.h    |  2 +
 net/core/skbuff.c         | 95 +++++++++++++++++++++++++++++++++++++++++++++++
 net/openvswitch/actions.c | 86 +++++-------------------------------------
 3 files changed, 106 insertions(+), 77 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index e045516..78c299f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2679,6 +2679,8 @@ unsigned int skb_gso_transport_seglen(const struct sk_buff *skb);
 struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
 struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
 int skb_ensure_writable(struct sk_buff *skb, int write_len);
+int skb_vlan_pop(struct sk_buff *skb);
+int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
 
 struct skb_checksum_ops {
 	__wsum (*update)(const void *mem, int len, __wsum wsum);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d11bbe0..c906c5f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4163,6 +4163,101 @@ int skb_ensure_writable(struct sk_buff *skb, int write_len)
 }
 EXPORT_SYMBOL(skb_ensure_writable);
 
+/* remove VLAN header from packet and update csum accordingly. */
+static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
+{
+	struct vlan_hdr *vhdr;
+	unsigned int offset = skb->data - skb_mac_header(skb);
+	int err;
+
+	__skb_push(skb, offset);
+	err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
+	if (unlikely(err))
+		goto pull;
+
+	skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
+
+	vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
+	*vlan_tci = ntohs(vhdr->h_vlan_TCI);
+
+	memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
+	__skb_pull(skb, VLAN_HLEN);
+
+	vlan_set_encap_proto(skb, vhdr);
+	skb->mac_header += VLAN_HLEN;
+
+	if (skb_network_offset(skb) < ETH_HLEN)
+		skb_set_network_header(skb, ETH_HLEN);
+
+	skb_reset_mac_len(skb);
+pull:
+	__skb_pull(skb, offset);
+
+	return err;
+}
+
+int skb_vlan_pop(struct sk_buff *skb)
+{
+	u16 vlan_tci;
+	__be16 vlan_proto;
+	int err;
+
+	if (likely(vlan_tx_tag_present(skb))) {
+		skb->vlan_tci = 0;
+	} else {
+		if (unlikely((skb->protocol != htons(ETH_P_8021Q) &&
+			      skb->protocol != htons(ETH_P_8021AD)) ||
+			     skb->len < VLAN_ETH_HLEN))
+			return 0;
+
+		err = __skb_vlan_pop(skb, &vlan_tci);
+		if (err)
+			return err;
+	}
+	/* move next vlan tag to hw accel tag */
+	if (likely((skb->protocol != htons(ETH_P_8021Q) &&
+		    skb->protocol != htons(ETH_P_8021AD)) ||
+		   skb->len < VLAN_ETH_HLEN))
+		return 0;
+
+	vlan_proto = skb->protocol;
+	err = __skb_vlan_pop(skb, &vlan_tci);
+	if (unlikely(err))
+		return err;
+
+	__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+	return 0;
+}
+EXPORT_SYMBOL(skb_vlan_pop);
+
+int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
+{
+	if (vlan_tx_tag_present(skb)) {
+		unsigned int offset = skb->data - skb_mac_header(skb);
+		int err;
+
+		/* __vlan_insert_tag expect skb->data pointing to mac header.
+		 * So change skb->data before calling it and change back to
+		 * original position later
+		 */
+		__skb_push(skb, offset);
+		err = __vlan_insert_tag(skb, skb->vlan_proto,
+					vlan_tx_tag_get(skb));
+		if (err)
+			return err;
+		skb->protocol = skb->vlan_proto;
+		skb->mac_len += VLAN_HLEN;
+		__skb_pull(skb, offset);
+
+		if (skb->ip_summed == CHECKSUM_COMPLETE)
+			skb->csum = csum_add(skb->csum, csum_partial(skb->data
+					+ (2 * ETH_ALEN), VLAN_HLEN, 0));
+	}
+	__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+	return 0;
+}
+EXPORT_SYMBOL(skb_vlan_push);
+
 /**
  * alloc_skb_with_frags - allocate skb with page frags
  *
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 7ffa377..4e05ea1 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -206,93 +206,27 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	return 0;
 }
 
-/* remove VLAN header from packet and update csum accordingly. */
-static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
-{
-	struct vlan_hdr *vhdr;
-	int err;
-
-	err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
-	if (unlikely(err))
-		return err;
-
-	skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
-
-	vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
-	*current_tci = vhdr->h_vlan_TCI;
-
-	memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
-	__skb_pull(skb, VLAN_HLEN);
-
-	vlan_set_encap_proto(skb, vhdr);
-	skb->mac_header += VLAN_HLEN;
-
-	if (skb_network_offset(skb) < ETH_HLEN)
-		skb_set_network_header(skb, ETH_HLEN);
-
-	/* Update mac_len for subsequent MPLS actions */
-	skb_reset_mac_len(skb);
-	return 0;
-}
-
 static int pop_vlan(struct sk_buff *skb, struct sw_flow_key *key)
 {
-	__be16 tci;
 	int err;
 
-	if (likely(vlan_tx_tag_present(skb))) {
-		skb->vlan_tci = 0;
-	} else {
-		if (unlikely(skb->protocol != htons(ETH_P_8021Q) ||
-			     skb->len < VLAN_ETH_HLEN))
-			return 0;
-
-		err = __pop_vlan_tci(skb, &tci);
-		if (err)
-			return err;
-	}
-	/* move next vlan tag to hw accel tag */
-	if (likely(skb->protocol != htons(ETH_P_8021Q) ||
-		   skb->len < VLAN_ETH_HLEN)) {
+	err = skb_vlan_pop(skb);
+	if (vlan_tx_tag_present(skb))
+		invalidate_flow_key(key);
+	else
 		key->eth.tci = 0;
-		return 0;
-	}
-
-	invalidate_flow_key(key);
-	err = __pop_vlan_tci(skb, &tci);
-	if (unlikely(err))
-		return err;
-
-	__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(tci));
-	return 0;
+	return err;
 }
 
 static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
 		     const struct ovs_action_push_vlan *vlan)
 {
-	if (unlikely(vlan_tx_tag_present(skb))) {
-		u16 current_tag;
-
-		/* push down current VLAN tag */
-		current_tag = vlan_tx_tag_get(skb);
-
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						current_tag);
-		if (!skb)
-			return -ENOMEM;
-		/* Update mac_len for subsequent MPLS actions */
-		skb->mac_len += VLAN_HLEN;
-
-		if (skb->ip_summed == CHECKSUM_COMPLETE)
-			skb->csum = csum_add(skb->csum, csum_partial(skb->data
-					+ (2 * ETH_ALEN), VLAN_HLEN, 0));
-
+	if (vlan_tx_tag_present(skb))
 		invalidate_flow_key(key);
-	} else {
+	else
 		key->eth.tci = vlan->vlan_tci;
-	}
-	__vlan_hwaccel_put_tag(skb, vlan->vlan_tpid, ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
-	return 0;
+	return skb_vlan_push(skb, vlan->vlan_tpid,
+			     ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
 }
 
 static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
@@ -858,8 +792,6 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
 
 		case OVS_ACTION_ATTR_PUSH_VLAN:
 			err = push_vlan(skb, key, nla_data(a));
-			if (unlikely(err)) /* skb already freed. */
-				return err;
 			break;
 
 		case OVS_ACTION_ATTR_POP_VLAN:
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v4 7/9] net: move make_writable helper into common code
From: Jiri Pirko @ 2014-11-19 13:05 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

note that skb_make_writable already exists in net/netfilter/core.c
but does something slightly different.

Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
---
 include/linux/skbuff.h    |  1 +
 net/core/skbuff.c         | 12 ++++++++++++
 net/openvswitch/actions.c | 39 ++++++++++++++-------------------------
 3 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 73c370e..e045516 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2678,6 +2678,7 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet);
 unsigned int skb_gso_transport_seglen(const struct sk_buff *skb);
 struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
 struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
+int skb_ensure_writable(struct sk_buff *skb, int write_len);
 
 struct skb_checksum_ops {
 	__wsum (*update)(const void *mem, int len, __wsum wsum);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7001896..d11bbe0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4151,6 +4151,18 @@ err_free:
 }
 EXPORT_SYMBOL(skb_vlan_untag);
 
+int skb_ensure_writable(struct sk_buff *skb, int write_len)
+{
+	if (!pskb_may_pull(skb, write_len))
+		return -ENOMEM;
+
+	if (!skb_cloned(skb) || skb_clone_writable(skb, write_len))
+		return 0;
+
+	return pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
+}
+EXPORT_SYMBOL(skb_ensure_writable);
+
 /**
  * alloc_skb_with_frags - allocate skb with page frags
  *
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 426b913..7ffa377 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -119,17 +119,6 @@ static bool is_flow_key_valid(const struct sw_flow_key *key)
 	return !!key->eth.type;
 }
 
-static int make_writable(struct sk_buff *skb, int write_len)
-{
-	if (!pskb_may_pull(skb, write_len))
-		return -ENOMEM;
-
-	if (!skb_cloned(skb) || skb_clone_writable(skb, write_len))
-		return 0;
-
-	return pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
-}
-
 static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 		     const struct ovs_action_push_mpls *mpls)
 {
@@ -171,7 +160,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	struct ethhdr *hdr;
 	int err;
 
-	err = make_writable(skb, skb->mac_len + MPLS_HLEN);
+	err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
 	if (unlikely(err))
 		return err;
 
@@ -201,7 +190,7 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	__be32 *stack;
 	int err;
 
-	err = make_writable(skb, skb->mac_len + MPLS_HLEN);
+	err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
 	if (unlikely(err))
 		return err;
 
@@ -223,7 +212,7 @@ static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
 	struct vlan_hdr *vhdr;
 	int err;
 
-	err = make_writable(skb, VLAN_ETH_HLEN);
+	err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
 	if (unlikely(err))
 		return err;
 
@@ -310,7 +299,7 @@ static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
 			const struct ovs_key_ethernet *eth_key)
 {
 	int err;
-	err = make_writable(skb, ETH_HLEN);
+	err = skb_ensure_writable(skb, ETH_HLEN);
 	if (unlikely(err))
 		return err;
 
@@ -412,8 +401,8 @@ static int set_ipv4(struct sk_buff *skb, struct sw_flow_key *key,
 	struct iphdr *nh;
 	int err;
 
-	err = make_writable(skb, skb_network_offset(skb) +
-				 sizeof(struct iphdr));
+	err = skb_ensure_writable(skb, skb_network_offset(skb) +
+				  sizeof(struct iphdr));
 	if (unlikely(err))
 		return err;
 
@@ -450,8 +439,8 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *key,
 	__be32 *saddr;
 	__be32 *daddr;
 
-	err = make_writable(skb, skb_network_offset(skb) +
-			    sizeof(struct ipv6hdr));
+	err = skb_ensure_writable(skb, skb_network_offset(skb) +
+				  sizeof(struct ipv6hdr));
 	if (unlikely(err))
 		return err;
 
@@ -493,7 +482,7 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *key,
 	return 0;
 }
 
-/* Must follow make_writable() since that can move the skb data. */
+/* Must follow skb_ensure_writable() since that can move the skb data. */
 static void set_tp_port(struct sk_buff *skb, __be16 *port,
 			 __be16 new_port, __sum16 *check)
 {
@@ -523,8 +512,8 @@ static int set_udp(struct sk_buff *skb, struct sw_flow_key *key,
 	struct udphdr *uh;
 	int err;
 
-	err = make_writable(skb, skb_transport_offset(skb) +
-				 sizeof(struct udphdr));
+	err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+				  sizeof(struct udphdr));
 	if (unlikely(err))
 		return err;
 
@@ -548,8 +537,8 @@ static int set_tcp(struct sk_buff *skb, struct sw_flow_key *key,
 	struct tcphdr *th;
 	int err;
 
-	err = make_writable(skb, skb_transport_offset(skb) +
-				 sizeof(struct tcphdr));
+	err = skb_ensure_writable(skb, skb_transport_offset(skb) +
+				  sizeof(struct tcphdr));
 	if (unlikely(err))
 		return err;
 
@@ -574,7 +563,7 @@ static int set_sctp(struct sk_buff *skb, struct sw_flow_key *key,
 	int err;
 	unsigned int sctphoff = skb_transport_offset(skb);
 
-	err = make_writable(skb, sctphoff + sizeof(struct sctphdr));
+	err = skb_ensure_writable(skb, sctphoff + sizeof(struct sctphdr));
 	if (unlikely(err))
 		return err;
 
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v4 6/9] vlan: introduce __vlan_insert_tag helper which does not free skb
From: Jiri Pirko @ 2014-11-19 13:05 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

There's a need for helper which inserts vlan tag but does not free the
skb in case of an error.

Suggested-by: Pravin Shelar <pshelar@nicira.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
---
 include/linux/if_vlan.h | 45 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 291e670..515a35e 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -282,28 +282,24 @@ static inline bool vlan_hw_offload_capable(netdev_features_t features,
 }
 
 /**
- * vlan_insert_tag - regular VLAN tag inserting
+ * __vlan_insert_tag - regular VLAN tag inserting
  * @skb: skbuff to tag
  * @vlan_proto: VLAN encapsulation protocol
  * @vlan_tci: VLAN TCI to insert
  *
  * Inserts the VLAN tag into @skb as part of the payload
- * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
- *
- * Following the skb_unshare() example, in case of error, the calling function
- * doesn't have to worry about freeing the original skb.
+ * Returns error if skb_cow_head failes.
  *
  * Does not change skb->protocol so this function can be used during receive.
  */
-static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
-					      __be16 vlan_proto, u16 vlan_tci)
+static inline int __vlan_insert_tag(struct sk_buff *skb,
+				    __be16 vlan_proto, u16 vlan_tci)
 {
 	struct vlan_ethhdr *veth;
 
-	if (skb_cow_head(skb, VLAN_HLEN) < 0) {
-		dev_kfree_skb_any(skb);
-		return NULL;
-	}
+	if (skb_cow_head(skb, VLAN_HLEN) < 0)
+		return -ENOMEM;
+
 	veth = (struct vlan_ethhdr *)skb_push(skb, VLAN_HLEN);
 
 	/* Move the mac addresses to the beginning of the new header. */
@@ -316,6 +312,33 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
 	/* now, the TCI */
 	veth->h_vlan_TCI = htons(vlan_tci);
 
+	return 0;
+}
+
+/**
+ * vlan_insert_tag - regular VLAN tag inserting
+ * @skb: skbuff to tag
+ * @vlan_proto: VLAN encapsulation protocol
+ * @vlan_tci: VLAN TCI to insert
+ *
+ * Inserts the VLAN tag into @skb as part of the payload
+ * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ *
+ * Does not change skb->protocol so this function can be used during receive.
+ */
+static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
+					      __be16 vlan_proto, u16 vlan_tci)
+{
+	int err;
+
+	err = __vlan_insert_tag(skb, vlan_proto, vlan_tci);
+	if (err) {
+		dev_kfree_skb_any(skb);
+		return NULL;
+	}
 	return skb;
 }
 
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v4 5/9] vlan: introduce *vlan_hwaccel_push_inside helpers
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

Use them to push skb->vlan_tci into the payload and avoid code
duplication.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---

v3->v4:
- fixed error path in ovs datapath

 drivers/net/vxlan.c         | 22 ++++++----------------
 include/linux/if_vlan.h     | 34 ++++++++++++++++++++++++++++++++++
 net/core/dev.c              |  8 ++------
 net/core/netpoll.c          |  4 +---
 net/ipv4/geneve.c           | 11 +++--------
 net/openvswitch/datapath.c  |  4 +---
 net/openvswitch/vport-gre.c | 12 ++++--------
 7 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index bb8fbab..64d45fa 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1599,14 +1599,9 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
 	if (unlikely(err))
 		return err;
 
-	if (vlan_tx_tag_present(skb)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
-		if (WARN_ON(!skb))
-			return -ENOMEM;
-
-		skb->vlan_tci = 0;
-	}
+	skb = vlan_hwaccel_push_inside(skb);
+	if (WARN_ON(!skb))
+		return -ENOMEM;
 
 	vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
 	vxh->vx_flags = htonl(VXLAN_FLAGS);
@@ -1643,14 +1638,9 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
 	if (unlikely(err))
 		return err;
 
-	if (vlan_tx_tag_present(skb)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
-		if (WARN_ON(!skb))
-			return -ENOMEM;
-
-		skb->vlan_tci = 0;
-	}
+	skb = vlan_hwaccel_push_inside(skb);
+	if (WARN_ON(!skb))
+		return -ENOMEM;
 
 	vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
 	vxh->vx_flags = htonl(VXLAN_FLAGS);
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 46e4a15..291e670 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -341,6 +341,40 @@ static inline struct sk_buff *vlan_insert_tag_set_proto(struct sk_buff *skb,
 	return skb;
 }
 
+/*
+ * __vlan_hwaccel_push_inside - pushes vlan tag to the payload
+ * @skb: skbuff to tag
+ *
+ * Pushes the VLAN tag from @skb->vlan_tci inside to the payload.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ */
+static inline struct sk_buff *__vlan_hwaccel_push_inside(struct sk_buff *skb)
+{
+	skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+					vlan_tx_tag_get(skb));
+	if (likely(skb))
+		skb->vlan_tci = 0;
+	return skb;
+}
+/*
+ * vlan_hwaccel_push_inside - pushes vlan tag to the payload
+ * @skb: skbuff to tag
+ *
+ * Checks is tag is present in @skb->vlan_tci and if it is, it pushes the
+ * VLAN tag from @skb->vlan_tci inside to the payload.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ */
+static inline struct sk_buff *vlan_hwaccel_push_inside(struct sk_buff *skb)
+{
+	if (vlan_tx_tag_present(skb))
+		skb = __vlan_hwaccel_push_inside(skb);
+	return skb;
+}
+
 /**
  * __vlan_hwaccel_put_tag - hardware accelerated VLAN inserting
  * @skb: skbuff to tag
diff --git a/net/core/dev.c b/net/core/dev.c
index 3611e60..ac48362 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2644,12 +2644,8 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
 					  netdev_features_t features)
 {
 	if (vlan_tx_tag_present(skb) &&
-	    !vlan_hw_offload_capable(features, skb->vlan_proto)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
-		if (skb)
-			skb->vlan_tci = 0;
-	}
+	    !vlan_hw_offload_capable(features, skb->vlan_proto))
+		skb = __vlan_hwaccel_push_inside(skb);
 	return skb;
 }
 
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 65d3723..e0ad5d1 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -79,8 +79,7 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
 
 	if (vlan_tx_tag_present(skb) &&
 	    !vlan_hw_offload_capable(features, skb->vlan_proto)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
+		skb = __vlan_hwaccel_push_inside(skb);
 		if (unlikely(!skb)) {
 			/* This is actually a packet drop, but we
 			 * don't want the code that calls this
@@ -88,7 +87,6 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			 */
 			goto out;
 		}
-		skb->vlan_tci = 0;
 	}
 
 	status = netdev_start_xmit(skb, dev, txq, false);
diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
index fd430a6..a457232 100644
--- a/net/ipv4/geneve.c
+++ b/net/ipv4/geneve.c
@@ -131,14 +131,9 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
 	if (unlikely(err))
 		return err;
 
-	if (vlan_tx_tag_present(skb)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
-		if (unlikely(!skb)
-			return -ENOMEM;
-
-		skb->vlan_tci = 0;
-	}
+	skb = vlan_hwaccel_push_inside(skb);
+	if (unlikely(!skb))
+		return -ENOMEM;
 
 	gnvh = (struct genevehdr *)__skb_push(skb, sizeof(*gnvh) + opt_len);
 	geneve_build_header(gnvh, tun_flags, vni, opt_len, opt);
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index c63e60e..f37ca3e 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -425,12 +425,10 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
 		if (!nskb)
 			return -ENOMEM;
 
-		nskb = vlan_insert_tag_set_proto(nskb, nskb->vlan_proto,
-						 vlan_tx_tag_get(nskb));
+		nskb = __vlan_hwaccel_push_inside(nskb);
 		if (!nskb)
 			return -ENOMEM;
 
-		nskb->vlan_tci = 0;
 		skb = nskb;
 	}
 
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index 777cd8c..6b69df5 100644
--- a/net/openvswitch/vport-gre.c
+++ b/net/openvswitch/vport-gre.c
@@ -175,14 +175,10 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 			goto err_free_rt;
 	}
 
-	if (vlan_tx_tag_present(skb)) {
-		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
-						vlan_tx_tag_get(skb));
-		if (unlikely(!skb) {
-			err = -ENOMEM;
-			goto err_free_rt;
-		}
-		skb->vlan_tci = 0;
+	skb = vlan_hwaccel_push_inside(skb);
+	if (unlikely(!skb)) {
+		err = -ENOMEM;
+		goto err_free_rt;
 	}
 
 	/* Push Tunnel header. */
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v4 4/9] vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

Name fits better. Plus there's going to be introduced
__vlan_insert_tag later on.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
---
 drivers/net/bonding/bond_main.c             |  4 ++--
 drivers/net/ethernet/emulex/benet/be_main.c |  6 ++++--
 drivers/net/vxlan.c                         | 12 ++++++------
 include/linux/if_vlan.h                     |  8 +++++---
 net/bridge/br_vlan.c                        |  4 ++--
 net/core/dev.c                              |  4 ++--
 net/core/netpoll.c                          |  4 ++--
 net/ipv4/geneve.c                           | 11 +++++------
 net/openvswitch/actions.c                   |  4 +++-
 net/openvswitch/datapath.c                  |  3 ++-
 net/openvswitch/vport-gre.c                 |  6 +++---
 11 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e26c682..c1d7da4 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2146,8 +2146,8 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op,
 
 		netdev_dbg(slave_dev, "inner tag: proto %X vid %X\n",
 			   ntohs(outer_tag->vlan_proto), tags->vlan_id);
-		skb = __vlan_put_tag(skb, tags->vlan_proto,
-				     tags->vlan_id);
+		skb = vlan_insert_tag_set_proto(skb, tags->vlan_proto,
+						tags->vlan_id);
 		if (!skb) {
 			net_err_ratelimited("failed to insert inner VLAN tag\n");
 			return;
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 54160cc..d02fbc7 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -887,7 +887,8 @@ static struct sk_buff *be_insert_vlan_in_pkt(struct be_adapter *adapter,
 	}
 
 	if (vlan_tag) {
-		skb = __vlan_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
+		skb = vlan_insert_tag_set_proto(skb, htons(ETH_P_8021Q),
+						vlan_tag);
 		if (unlikely(!skb))
 			return skb;
 		skb->vlan_tci = 0;
@@ -896,7 +897,8 @@ static struct sk_buff *be_insert_vlan_in_pkt(struct be_adapter *adapter,
 	/* Insert the outer VLAN, if any */
 	if (adapter->qnq_vid) {
 		vlan_tag = adapter->qnq_vid;
-		skb = __vlan_put_tag(skb, htons(ETH_P_8021Q), vlan_tag);
+		skb = vlan_insert_tag_set_proto(skb, htons(ETH_P_8021Q),
+						vlan_tag);
 		if (unlikely(!skb))
 			return skb;
 		if (skip_hw_vlan)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 23b1e8c..bb8fbab 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1600,9 +1600,9 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
 		return err;
 
 	if (vlan_tx_tag_present(skb)) {
-		if (WARN_ON(!__vlan_put_tag(skb,
-					    skb->vlan_proto,
-					    vlan_tx_tag_get(skb))))
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
+		if (WARN_ON(!skb))
 			return -ENOMEM;
 
 		skb->vlan_tci = 0;
@@ -1644,9 +1644,9 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
 		return err;
 
 	if (vlan_tx_tag_present(skb)) {
-		if (WARN_ON(!__vlan_put_tag(skb,
-					    skb->vlan_proto,
-					    vlan_tx_tag_get(skb))))
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
+		if (WARN_ON(!skb))
 			return -ENOMEM;
 
 		skb->vlan_tci = 0;
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 75b70a5..46e4a15 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -320,8 +320,9 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
 }
 
 /**
- * __vlan_put_tag - regular VLAN tag inserting
+ * vlan_insert_tag_set_proto - regular VLAN tag inserting
  * @skb: skbuff to tag
+ * @vlan_proto: VLAN encapsulation protocol
  * @vlan_tci: VLAN TCI to insert
  *
  * Inserts the VLAN tag into @skb as part of the payload
@@ -330,8 +331,9 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,
  * Following the skb_unshare() example, in case of error, the calling function
  * doesn't have to worry about freeing the original skb.
  */
-static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb,
-					     __be16 vlan_proto, u16 vlan_tci)
+static inline struct sk_buff *vlan_insert_tag_set_proto(struct sk_buff *skb,
+							__be16 vlan_proto,
+							u16 vlan_tci)
 {
 	skb = vlan_insert_tag(skb, vlan_proto, vlan_tci);
 	if (skb)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 150048f..97b8ddf 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -199,8 +199,8 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
 		if (skb->vlan_proto != proto) {
 			/* Protocol-mismatch, empty out vlan_tci for new tag */
 			skb_push(skb, ETH_HLEN);
-			skb = __vlan_put_tag(skb, skb->vlan_proto,
-					     vlan_tx_tag_get(skb));
+			skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+							vlan_tx_tag_get(skb));
 			if (unlikely(!skb))
 				return false;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 1ab168e..3611e60 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2645,8 +2645,8 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
 {
 	if (vlan_tx_tag_present(skb) &&
 	    !vlan_hw_offload_capable(features, skb->vlan_proto)) {
-		skb = __vlan_put_tag(skb, skb->vlan_proto,
-				     vlan_tx_tag_get(skb));
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
 		if (skb)
 			skb->vlan_tci = 0;
 	}
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index e6645b4..65d3723 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -79,8 +79,8 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
 
 	if (vlan_tx_tag_present(skb) &&
 	    !vlan_hw_offload_capable(features, skb->vlan_proto)) {
-		skb = __vlan_put_tag(skb, skb->vlan_proto,
-				     vlan_tx_tag_get(skb));
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
 		if (unlikely(!skb)) {
 			/* This is actually a packet drop, but we
 			 * don't want the code that calls this
diff --git a/net/ipv4/geneve.c b/net/ipv4/geneve.c
index 31802af..fd430a6 100644
--- a/net/ipv4/geneve.c
+++ b/net/ipv4/geneve.c
@@ -132,12 +132,11 @@ int geneve_xmit_skb(struct geneve_sock *gs, struct rtable *rt,
 		return err;
 
 	if (vlan_tx_tag_present(skb)) {
-		if (unlikely(!__vlan_put_tag(skb,
-					     skb->vlan_proto,
-					     vlan_tx_tag_get(skb)))) {
-			err = -ENOMEM;
-			return err;
-		}
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
+		if (unlikely(!skb)
+			return -ENOMEM;
+
 		skb->vlan_tci = 0;
 	}
 
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 749a301..426b913 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -287,7 +287,9 @@ static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
 		/* push down current VLAN tag */
 		current_tag = vlan_tx_tag_get(skb);
 
-		if (!__vlan_put_tag(skb, skb->vlan_proto, current_tag))
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						current_tag);
+		if (!skb)
 			return -ENOMEM;
 		/* Update mac_len for subsequent MPLS actions */
 		skb->mac_len += VLAN_HLEN;
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index ab141d4..c63e60e 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -425,7 +425,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
 		if (!nskb)
 			return -ENOMEM;
 
-		nskb = __vlan_put_tag(nskb, nskb->vlan_proto, vlan_tx_tag_get(nskb));
+		nskb = vlan_insert_tag_set_proto(nskb, nskb->vlan_proto,
+						 vlan_tx_tag_get(nskb));
 		if (!nskb)
 			return -ENOMEM;
 
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index 8e61a5c..777cd8c 100644
--- a/net/openvswitch/vport-gre.c
+++ b/net/openvswitch/vport-gre.c
@@ -176,9 +176,9 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
 	}
 
 	if (vlan_tx_tag_present(skb)) {
-		if (unlikely(!__vlan_put_tag(skb,
-					     skb->vlan_proto,
-					     vlan_tx_tag_get(skb)))) {
+		skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
+						vlan_tx_tag_get(skb));
+		if (unlikely(!skb) {
 			err = -ENOMEM;
 			goto err_free_rt;
 		}
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v4 3/9] vlan: kill vlan_put_tag helper
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

Since both tx and rx paths work with skb->vlan_tci, there's no need for
this function anymore. Switch users directly to __vlan_hwaccel_put_tag.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/bonding/bond_alb.c  | 17 ++++-------------
 drivers/net/bonding/bond_main.c |  8 ++------
 drivers/net/usb/cdc_mbim.c      |  2 +-
 include/linux/if_vlan.h         | 19 -------------------
 4 files changed, 7 insertions(+), 39 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index e1f1a00..bb9e9fc 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -475,12 +475,8 @@ static void rlb_update_client(struct rlb_client_info *client_info)
 		skb->dev = client_info->slave->dev;
 
 		if (client_info->vlan_id) {
-			skb = vlan_put_tag(skb, htons(ETH_P_8021Q), client_info->vlan_id);
-			if (!skb) {
-				netdev_err(client_info->slave->bond->dev,
-					   "failed to insert VLAN tag\n");
-				continue;
-			}
+			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
+					       client_info->vlan_id);
 		}
 
 		arp_xmit(skb);
@@ -951,13 +947,8 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
 	skb->priority = TC_PRIO_CONTROL;
 	skb->dev = slave->dev;
 
-	if (vid) {
-		skb = vlan_put_tag(skb, vlan_proto, vid);
-		if (!skb) {
-			netdev_err(slave->bond->dev, "failed to insert VLAN tag\n");
-			return;
-		}
-	}
+	if (vid)
+		__vlan_hwaccel_put_tag(skb, vlan_proto, vid);
 
 	dev_queue_xmit(skb);
 }
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8575fee..e26c682 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2159,12 +2159,8 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op,
 	if (outer_tag->vlan_id) {
 		netdev_dbg(slave_dev, "outer tag: proto %X vid %X\n",
 			   ntohs(outer_tag->vlan_proto), outer_tag->vlan_id);
-		skb = vlan_put_tag(skb, outer_tag->vlan_proto,
-				   outer_tag->vlan_id);
-		if (!skb) {
-			net_err_ratelimited("failed to insert outer VLAN tag\n");
-			return;
-		}
+		__vlan_hwaccel_put_tag(skb, outer_tag->vlan_proto,
+				       outer_tag->vlan_id);
 	}
 
 xmit:
diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
index 5ee7a1d..96fc8a5 100644
--- a/drivers/net/usb/cdc_mbim.c
+++ b/drivers/net/usb/cdc_mbim.c
@@ -402,7 +402,7 @@ static struct sk_buff *cdc_mbim_process_dgram(struct usbnet *dev, u8 *buf, size_
 
 	/* map MBIM session to VLAN */
 	if (tci)
-		vlan_put_tag(skb, htons(ETH_P_8021Q), tci);
+		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), tci);
 err:
 	return skb;
 }
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 1b5dbc2..75b70a5 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -355,25 +355,6 @@ static inline void __vlan_hwaccel_put_tag(struct sk_buff *skb,
 }
 
 /**
- * vlan_put_tag - inserts VLAN tag according to device features
- * @skb: skbuff to tag
- * @vlan_tci: VLAN TCI to insert
- *
- * Assumes skb->dev is the target that will xmit this frame.
- * Returns a VLAN tagged skb.
- */
-static inline struct sk_buff *vlan_put_tag(struct sk_buff *skb,
-					   __be16 vlan_proto, u16 vlan_tci)
-{
-	if (vlan_hw_offload_capable(skb->dev->features, vlan_proto)) {
-		__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
-		return skb;
-	} else {
-		return __vlan_put_tag(skb, vlan_proto, vlan_tci);
-	}
-}
-
-/**
  * __vlan_get_tag - get the VLAN ID that is part of the payload
  * @skb: skbuff to query
  * @vlan_tci: buffer to store vlaue
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v4 2/9] vlan: make __vlan_hwaccel_put_tag return void
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

Always returns the same skb it gets, so change to void.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
---
 drivers/scsi/fcoe/fcoe.c | 6 ++----
 include/linux/if_vlan.h  | 9 ++++-----
 net/8021q/vlan_dev.c     | 2 +-
 3 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 4a8ac7d..73a8cc4 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1669,10 +1669,8 @@ static int fcoe_xmit(struct fc_lport *lport, struct fc_frame *fp)
 	    fcoe->realdev->features & NETIF_F_HW_VLAN_CTAG_TX) {
 		/* must set skb->dev before calling vlan_put_tag */
 		skb->dev = fcoe->realdev;
-		skb = __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
-					     vlan_dev_vlan_id(fcoe->netdev));
-		if (!skb)
-			return -ENOMEM;
+		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
+				       vlan_dev_vlan_id(fcoe->netdev));
 	} else
 		skb->dev = fcoe->netdev;
 
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index d69f057..1b5dbc2 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -347,13 +347,11 @@ static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb,
  *
  * Puts the VLAN TCI in @skb->vlan_tci and lets the device do the rest
  */
-static inline struct sk_buff *__vlan_hwaccel_put_tag(struct sk_buff *skb,
-						     __be16 vlan_proto,
-						     u16 vlan_tci)
+static inline void __vlan_hwaccel_put_tag(struct sk_buff *skb,
+					  __be16 vlan_proto, u16 vlan_tci)
 {
 	skb->vlan_proto = vlan_proto;
 	skb->vlan_tci = VLAN_TAG_PRESENT | vlan_tci;
-	return skb;
 }
 
 /**
@@ -368,7 +366,8 @@ static inline struct sk_buff *vlan_put_tag(struct sk_buff *skb,
 					   __be16 vlan_proto, u16 vlan_tci)
 {
 	if (vlan_hw_offload_capable(skb->dev->features, vlan_proto)) {
-		return __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+		__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+		return skb;
 	} else {
 		return __vlan_put_tag(skb, vlan_proto, vlan_tci);
 	}
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 0d441ec..d6524b2 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -150,7 +150,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 		u16 vlan_tci;
 		vlan_tci = vlan->vlan_id;
 		vlan_tci |= vlan_dev_get_egress_qos_mask(dev, skb->priority);
-		skb = __vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
+		__vlan_hwaccel_put_tag(skb, vlan->vlan_proto, vlan_tci);
 	}
 
 	skb->dev = vlan->real_dev;
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v4 1/9] openvswitch: actions: use skb_postpull_rcsum when possible
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

Replace duplicated code by calling skb_postpull_rcsum

Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/openvswitch/actions.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 394efa6..749a301 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -175,10 +175,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	if (unlikely(err))
 		return err;
 
-	if (skb->ip_summed == CHECKSUM_COMPLETE)
-		skb->csum = csum_sub(skb->csum,
-				     csum_partial(skb_mpls_header(skb),
-						  MPLS_HLEN, 0));
+	skb_postpull_rcsum(skb, skb_mpls_header(skb), MPLS_HLEN);
 
 	memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
 		skb->mac_len);
@@ -230,9 +227,7 @@ static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
 	if (unlikely(err))
 		return err;
 
-	if (skb->ip_summed == CHECKSUM_COMPLETE)
-		skb->csum = csum_sub(skb->csum, csum_partial(skb->data
-					+ (2 * ETH_ALEN), VLAN_HLEN, 0));
+	skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
 
 	vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
 	*current_tci = vhdr->h_vlan_TCI;
-- 
1.9.3

^ permalink raw reply related

* [patch net-next v4 0/9] sched: introduce vlan action
From: Jiri Pirko @ 2014-11-19 13:04 UTC (permalink / raw)
  To: netdev
  Cc: davem, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang

Please see the individual patches for info

Jiri Pirko (9):
  openvswitch: actions: use skb_postpull_rcsum when possible
  vlan: make __vlan_hwaccel_put_tag return void
  vlan: kill vlan_put_tag helper
  vlan: rename __vlan_put_tag to vlan_insert_tag_set_proto
  vlan: introduce *vlan_hwaccel_push_inside helpers
  vlan: introduce __vlan_insert_tag helper which does not free skb
  net: move make_writable helper into common code
  net: move vlan pop/push functions into common code
  sched: introduce vlan action

 drivers/net/bonding/bond_alb.c              |  17 +--
 drivers/net/bonding/bond_main.c             |  12 +-
 drivers/net/ethernet/emulex/benet/be_main.c |   6 +-
 drivers/net/usb/cdc_mbim.c                  |   2 +-
 drivers/net/vxlan.c                         |  22 +--
 drivers/scsi/fcoe/fcoe.c                    |   6 +-
 include/linux/if_vlan.h                     | 107 +++++++++-----
 include/linux/skbuff.h                      |   3 +
 include/net/tc_act/tc_vlan.h                |  27 ++++
 include/uapi/linux/tc_act/tc_vlan.h         |  35 +++++
 net/8021q/vlan_dev.c                        |   2 +-
 net/bridge/br_vlan.c                        |   4 +-
 net/core/dev.c                              |   8 +-
 net/core/netpoll.c                          |   4 +-
 net/core/skbuff.c                           | 107 ++++++++++++++
 net/ipv4/geneve.c                           |  12 +-
 net/openvswitch/actions.c                   | 128 ++++-------------
 net/openvswitch/datapath.c                  |   3 +-
 net/openvswitch/vport-gre.c                 |  12 +-
 net/sched/Kconfig                           |  11 ++
 net/sched/Makefile                          |   1 +
 net/sched/act_vlan.c                        | 207 ++++++++++++++++++++++++++++
 22 files changed, 522 insertions(+), 214 deletions(-)
 create mode 100644 include/net/tc_act/tc_vlan.h
 create mode 100644 include/uapi/linux/tc_act/tc_vlan.h
 create mode 100644 net/sched/act_vlan.c

-- 
1.9.3

^ permalink raw reply

* Re: [patch net-next v3 8/9] net: move vlan pop/push functions into common code
From: Jiri Benc @ 2014-11-19 13:03 UTC (permalink / raw)
  To: Pravin Shelar
  Cc: Jiri Pirko, netdev, David Miller, Jamal Hadi Salim, Tom Herbert,
	Eric Dumazet, Willem de Bruijn, Daniel Borkmann, mst, fw,
	Paul.Durrant, Thomas Graf, Cong Wang
In-Reply-To: <CALnjE+rJEtyL-QDrp0x+N_pCxDuR6soHkaY-OrswWrdZwfE2sQ@mail.gmail.com>

On Wed, 19 Nov 2014 00:06:19 -0800, Pravin Shelar wrote:
> skb_reset_mac_len() sets length according to ethernet and network
> offsets, but mpls expects mac-length to be offset to mpls header (ref.
> skb_mpls_header()). Therefore rather than reset we need to subtract
> VLAN_HLEN from mac_len. Same goes for vlan-push(), we can add
> VLAN_HLEN.

Subtracting VLAN_HLEN from mac_len would break the common vlan cases,
where skb->protocol is ETH_P_8021Q and skb->mac_len is 14 on input to
__skb_vlan_pop. After __skb_vlan_pop pops the vlan header, it changes
skb->protocol to the protocol of the actual frame and has to leave
skb->mac_len at 14.

You'll need something more sophisticated to support MPLS here, I'm
afraid.

 Jiri

-- 
Jiri Benc

^ permalink raw reply

* [PATCH] l2tp_eth: allow to set a specific mac address
From: Alexander Couzens @ 2014-11-19 12:24 UTC (permalink / raw)
  To: netdev; +Cc: Alexander Couzens

Signed-off-by: Alexander Couzens <lynxis@fe80.eu>
---
 net/l2tp/l2tp_eth.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index edb78e6..781b3a2 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -126,6 +126,7 @@ static struct net_device_ops l2tp_eth_netdev_ops = {
 	.ndo_uninit		= l2tp_eth_dev_uninit,
 	.ndo_start_xmit		= l2tp_eth_dev_xmit,
 	.ndo_get_stats64	= l2tp_eth_get_stats64,
+	.ndo_set_mac_address	= eth_mac_addr,
 };
 
 static void l2tp_eth_dev_setup(struct net_device *dev)
-- 
2.1.3

^ permalink raw reply related

* Re: [PATCH] net: team: expose sysfs attributes for each team option
From: Jiri Pirko @ 2014-11-19 12:41 UTC (permalink / raw)
  To: Hamad Kadmany; +Cc: netdev
In-Reply-To: <03067d01f18e264b30f86d0307d11b07.squirrel@www.codeaurora.org>

Wed, Nov 19, 2014 at 01:28:19PM CET, hkadmany@codeaurora.org wrote:
>On Mon, November 17, 2014 1:02 pm, Jiri Pirko wrote:
>>
>> Nak.
>>
>> I don't like this patch. The plan for team was keep things simple and to
>> have single userspace api using netlink, as that is the correct way to
>> deal with things. Sysfs for this use-case is not. Please, use netlink api.
>
>Using team driver requires using libnl3. In Android based platforms, libnl3 is not supported, and libnl3 license poses constraints in Android's user-space.

btw how about libteam? Does that poses the same constraints?


>
>This is the reason for the this change, to have at least the team options exposed through sysfs.
>
>
>>
>>>
>>>Signed-off-by: Hamad Kadmany <hkadmany@codeaurora.org>
>>>---
>>> drivers/net/team/team.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++--
>>> include/linux/if_team.h |   3 +
>>> 2 files changed, 278 insertions(+), 7 deletions(-)
>>>
>>>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>>>index 1222229..afd2f8f 100644
>>>--- a/drivers/net/team/team.c
>>>+++ b/drivers/net/team/team.c
>>>@@ -110,8 +110,198 @@ struct team_option_inst { /* One for each option instance */
>>> 	struct team_option_inst_info info;
>>> 	bool changed;
>>> 	bool removed;
>>>+	bool dev_attr_file_exist;
>>>+	struct device_attribute dev_attr; /* corresponding sysfs attribute */
>>> };
>>>
>>>+static struct attribute *team_sysfs_attrs[] = {
>>>+	NULL,
>>>+};
>>>+
>>>+static struct attribute_group team_sysfs_attr_group = {
>>>+	.attrs = team_sysfs_attrs,
>>>+};
>>>+
>>>+/* Device attributes (sysfs) */
>>>+
>>>+static ssize_t show_attribute(struct device *dev,
>>>+			      struct device_attribute *attr,
>>>+			      char *buf)
>>>+{
>>>+	struct team *team = dev_get_drvdata(dev);
>>>+	struct team_option_inst *opt_inst;
>>>+	ssize_t ret;
>>>+	struct team_option *option;
>>>+	struct team_gsetter_ctx ctx;
>>>+
>>>+	if (mutex_lock_interruptible(&team->lock))
>>>+		return -ERESTARTSYS;
>>>+
>>>+	opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>>+	option = opt_inst->option;
>>>+	if (!option->getter) {
>>>+		ret = -EOPNOTSUPP;
>>>+		netdev_err(team->dev,
>>>+			   "Option %s is write only\n", attr->attr.name);
>>>+		goto exit;
>>>+	}
>>>+
>>>+	ctx.info = &opt_inst->info;
>>>+	/* let the option getter do its job */
>>>+	ret = option->getter(team, &ctx);
>>>+	if (ret)
>>>+		goto exit;
>>>+
>>>+	/* translate option's output into sysfs output */
>>>+	switch (option->type) {
>>>+	case TEAM_OPTION_TYPE_U32:
>>>+		ret = scnprintf(buf, PAGE_SIZE, "%u\n", ctx.data.u32_val);
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_STRING:
>>>+		ret = scnprintf(buf, PAGE_SIZE, "%s\n", ctx.data.str_val);
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_BINARY:
>>>+		if (ctx.data.bin_val.len > PAGE_SIZE) {
>>>+			netdev_err(team->dev,
>>>+				   "Option output is too long (%d)\n",
>>>+				   ctx.data.bin_val.len);
>>>+			break;
>>>+		}
>>>+
>>>+		memcpy(buf, ctx.data.bin_val.ptr, ctx.data.bin_val.len);
>>>+		ret = ctx.data.bin_val.len;
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_BOOL:
>>>+		ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.bool_val);
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_S32:
>>>+		ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.s32_val);
>>>+		break;
>>>+	default:
>>>+		BUG();
>>>+	}
>>>+
>>>+exit:
>>>+	mutex_unlock(&team->lock);
>>>+
>>>+	return ret;
>>>+}
>>>+
>>>+static int team_nl_send_event_options_get(struct team *team,
>>>+					  struct list_head *sel_opt_inst_list);
>>>+
>>>+static ssize_t set_attribute(struct device *dev,
>>>+			     struct device_attribute *attr,
>>>+			     const char *buf, size_t count)
>>>+{
>>>+	struct team_option_inst *opt_inst;
>>>+	struct team *team = dev_get_drvdata(dev);
>>>+	int err = 0;
>>>+	struct team_option *option = NULL;
>>>+	struct team_gsetter_ctx ctx;
>>>+
>>>+	LIST_HEAD(opt_inst_list);
>>>+
>>>+	if (mutex_lock_interruptible(&team->lock))
>>>+		return -ERESTARTSYS;
>>>+
>>>+	opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>>+	option = opt_inst->option;
>>>+	if (!option->setter) {
>>>+		netdev_err(team->dev,
>>>+			   "Option %s is read only\n", attr->attr.name);
>>>+		err = -EOPNOTSUPP;
>>>+		goto exit;
>>>+	}
>>>+
>>>+	ctx.info = &opt_inst->info;
>>>+
>>>+	/* translate sysfs input into option's input */
>>>+	switch (option->type) {
>>>+	case TEAM_OPTION_TYPE_U32:
>>>+		err = kstrtoint(buf, 0, &ctx.data.u32_val);
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_STRING:
>>>+		if (count > TEAM_STRING_MAX_LEN) {
>>>+			netdev_err(team->dev,
>>>+				   "Input buffer too long (%zu)\n", count);
>>>+			err = -EINVAL;
>>>+			break;
>>>+		}
>>>+		ctx.data.str_val = buf;
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_BINARY:
>>>+		ctx.data.bin_val.len = count;
>>>+		ctx.data.bin_val.ptr = buf;
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_BOOL:
>>>+		err = strtobool(buf, &ctx.data.bool_val);
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_S32:
>>>+		err = kstrtoint(buf, 0, &ctx.data.s32_val);
>>>+		break;
>>>+	default:
>>>+		BUG();
>>>+	}
>>>+
>>>+	if (err) {
>>>+		netdev_err(team->dev, "Failed to translate input buffer\n");
>>>+		goto exit;
>>>+	}
>>>+
>>>+	/* let the option setter do its job */
>>>+	err = option->setter(team, &ctx);
>>>+	if (err)
>>>+		goto exit;
>>>+
>>>+	/* propagate option changed event */
>>>+	opt_inst->changed = true;
>>>+	list_add(&opt_inst->tmp_list, &opt_inst_list);
>>>+	err = team_nl_send_event_options_get(team, &opt_inst_list);
>>>+	if (err == -ESRCH) /* no listeners, not a real error */
>>>+		err = 0;
>>>+
>>>+exit:
>>>+	mutex_unlock(&team->lock);
>>>+
>>>+	if (!err)
>>>+		err = count;
>>>+	return err;
>>>+}
>>>+
>>>+/* create sysfs attribute for each option that is being registered */
>>>+static int __team_option_add_sysfs_attr(struct team *team,
>>>+					struct team_option_inst *opt_inst,
>>>+					bool create_sysfs_file)
>>>+{
>>>+	int err = 0;
>>>+	struct device_attribute *new_dev_attr = &opt_inst->dev_attr;
>>>+
>>>+	new_dev_attr->attr.name = opt_inst->option->name;
>>>+	new_dev_attr->attr.mode = S_IRUGO | S_IWUSR;
>>>+	new_dev_attr->show = show_attribute;
>>>+	new_dev_attr->store = set_attribute;
>>>+
>>>+	if (create_sysfs_file) {
>>>+		err = sysfs_create_file(&team->dev->dev.kobj,
>>>+					&new_dev_attr->attr);
>>>+		if (err)
>>>+			netdev_err(team->dev,
>>>+				   "Failed to create sysfs attribute %s\n",
>>>+				   new_dev_attr->attr.name);
>>>+	}
>>>+
>>>+	return err;
>>>+}
>>>+
>>>+static void __team_option_del_sysfs_attr(struct team *team,
>>>+					 struct team_option_inst *opt_inst)
>>>+{
>>>+	if (opt_inst->dev_attr_file_exist)
>>>+		sysfs_remove_file(&team->dev->dev.kobj,
>>>+				  &opt_inst->dev_attr.attr);
>>>+}
>>>+
>>> static struct team_option *__team_find_option(struct team *team,
>>> 					      const char *opt_name)
>>> {
>>>@@ -124,8 +314,10 @@ static struct team_option *__team_find_option(struct team *team,
>>> 	return NULL;
>>> }
>>>
>>>-static void __team_option_inst_del(struct team_option_inst *opt_inst)
>>>+static void __team_option_inst_del(struct team *team,
>>>+				   struct team_option_inst *opt_inst)
>>> {
>>>+	__team_option_del_sysfs_attr(team, opt_inst);
>>> 	list_del(&opt_inst->list);
>>> 	kfree(opt_inst);
>>> }
>>>@@ -137,7 +329,7 @@ static void __team_option_inst_del_option(struct team *team,
>>>
>>> 	list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>>> 		if (opt_inst->option == option)
>>>-			__team_option_inst_del(opt_inst);
>>>+			__team_option_inst_del(team, opt_inst);
>>> 	}
>>> }
>>>
>>>@@ -162,6 +354,7 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>>> 		opt_inst->info.array_index = i;
>>> 		opt_inst->changed = true;
>>> 		opt_inst->removed = false;
>>>+		opt_inst->dev_attr_file_exist = false;
>>> 		list_add_tail(&opt_inst->list, &team->option_inst_list);
>>> 		if (option->init) {
>>> 			err = option->init(team, &opt_inst->info);
>>>@@ -170,6 +363,20 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>>> 		}
>>>
>>> 	}
>>>+
>>>+	/* add sysfs attribute. per-port and array options are skipped */
>>>+	if (!option->per_port && !option->array_size) {
>>>+		/* create the sysfs file only if our state allows it */
>>>+		bool create_sysfs_file = device_is_registered(&team->dev->dev);
>>>+
>>>+		err = __team_option_add_sysfs_attr(team, opt_inst,
>>>+						   create_sysfs_file);
>>>+		if (err)
>>>+			return err;
>>>+
>>>+		opt_inst->dev_attr_file_exist = true;
>>>+	}
>>>+
>>> 	return 0;
>>> }
>>>
>>>@@ -218,7 +425,7 @@ static void __team_option_inst_del_port(struct team *team,
>>> 	list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>>> 		if (opt_inst->option->per_port &&
>>> 		    opt_inst->info.port == port)
>>>-			__team_option_inst_del(opt_inst);
>>>+			__team_option_inst_del(team, opt_inst);
>>> 	}
>>> }
>>>
>>>@@ -337,6 +544,51 @@ static void __team_options_unregister(struct team *team,
>>>
>>> static void __team_options_change_check(struct team *team);
>>>
>>>+static void team_attr_grp_free(struct team *team)
>>>+{
>>>+	kfree(team->attr_grp.attrs);
>>>+}
>>>+
>>>+/* allocate attribute group for creating sysfs for team's own options */
>>>+static int team_attr_grp_alloc(struct team *team)
>>>+{
>>>+	struct attribute **attributes;
>>>+	struct team_option_inst *opt_inst;
>>>+	int num_attr = 0;
>>>+	struct team_option *option;
>>>+
>>>+	list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>>+		option = opt_inst->option;
>>>+		/* per-port and array options currently not supported as
>>>+		 * sysfs attributes
>>>+		 */
>>>+		if (option->per_port || option->array_size)
>>>+			continue;
>>>+
>>>+		num_attr++;
>>>+	}
>>>+
>>>+	/* +1 for having NULL as last item in the array */
>>>+	attributes = kzalloc((num_attr + 1) * sizeof(*attributes), GFP_KERNEL);
>>>+	if (!attributes)
>>>+		return -ENOMEM;
>>>+	team->attr_grp.attrs = attributes;
>>>+
>>>+	num_attr = 0;
>>>+	list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>>+		option = opt_inst->option;
>>>+		/* per-port and array options currently not supported as
>>>+		 * sysfs attributes
>>>+		 */
>>>+		if (option->per_port || option->array_size)
>>>+			continue;
>>>+
>>>+		attributes[num_attr++] = &opt_inst->dev_attr.attr;
>>>+	}
>>>+
>>>+	return 0;
>>>+}
>>>+
>>> int team_options_register(struct team *team,
>>> 			  const struct team_option *option,
>>> 			  size_t option_count)
>>>@@ -1380,15 +1632,28 @@ static int team_init(struct net_device *dev)
>>>
>>> 	INIT_LIST_HEAD(&team->option_list);
>>> 	INIT_LIST_HEAD(&team->option_inst_list);
>>>-	err = team_options_register(team, team_options, ARRAY_SIZE(team_options));
>>>+
>>>+	err = team_options_register(team, team_options,
>>>+				    ARRAY_SIZE(team_options));
>>> 	if (err)
>>> 		goto err_options_register;
>>> 	netif_carrier_off(dev);
>>>
>>> 	team_set_lockdep_class(dev);
>>>
>>>+	/* store team context, to be used by Device attributes getter/setter */
>>>+	dev_set_drvdata(&dev->dev, team);
>>>+
>>>+	/* allocate and register sysfs attributes for team's own options */
>>>+	err = team_attr_grp_alloc(team);
>>>+	if (err)
>>>+		goto err_grp_alloc;
>>>+	dev->sysfs_groups[0] = &team->attr_grp;
>>>+
>>> 	return 0;
>>>
>>>+err_grp_alloc:
>>>+	team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>>> err_options_register:
>>> 	team_queue_override_fini(team);
>>> err_team_queue_override_init:
>>>@@ -1407,9 +1672,15 @@ static void team_uninit(struct net_device *dev)
>>> 	list_for_each_entry_safe(port, tmp, &team->port_list, list)
>>> 		team_port_del(team, port->dev);
>>>
>>>+	sysfs_remove_group(&team->dev->dev.kobj, &team->attr_grp);
>>>+	team_attr_grp_free(team);
>>>+	/* set to dummy group to avoid double free by core */
>>>+	dev->sysfs_groups[0] = &team_sysfs_attr_group;
>>>+
>>> 	__team_change_mode(team, NULL); /* cleanup */
>>> 	__team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>>> 	team_queue_override_fini(team);
>>>+
>>> 	mutex_unlock(&team->lock);
>>> }
>>>
>>>@@ -2194,9 +2465,6 @@ static int team_nl_cmd_options_get(struct sk_buff *skb, struct genl_info *info)
>>> 	return err;
>>> }
>>>
>>>-static int team_nl_send_event_options_get(struct team *team,
>>>-					  struct list_head *sel_opt_inst_list);
>>>-
>>> static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
>>> {
>>> 	struct team *team;
>>>diff --git a/include/linux/if_team.h b/include/linux/if_team.h
>>>index 25b8b15..2e9fb2a 100644
>>>--- a/include/linux/if_team.h
>>>+++ b/include/linux/if_team.h
>>>@@ -188,6 +188,9 @@ struct team {
>>> 	struct list_head option_list;
>>> 	struct list_head option_inst_list; /* list of option instances */
>>>
>>>+	/* attribute group for registering team's own options at init */
>>>+	struct attribute_group attr_grp;
>>>+
>>> 	const struct team_mode *mode;
>>> 	struct team_mode_ops ops;
>>> 	bool user_carrier_enabled;
>>>--
>>>1.8.5.2
>>>--
>>>Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
>>>The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>>>a Linux Foundation Collaborative Project
>>>
>>
>
>

^ permalink raw reply

* Re: [PATCH] net: team: expose sysfs attributes for each team option
From: Jiri Pirko @ 2014-11-19 12:40 UTC (permalink / raw)
  To: Hamad Kadmany; +Cc: netdev
In-Reply-To: <03067d01f18e264b30f86d0307d11b07.squirrel@www.codeaurora.org>

Wed, Nov 19, 2014 at 01:28:19PM CET, hkadmany@codeaurora.org wrote:
>On Mon, November 17, 2014 1:02 pm, Jiri Pirko wrote:
>>
>> Nak.
>>
>> I don't like this patch. The plan for team was keep things simple and to
>> have single userspace api using netlink, as that is the correct way to
>> deal with things. Sysfs for this use-case is not. Please, use netlink api.
>
>Using team driver requires using libnl3. In Android based platforms, libnl3 is not supported, and libnl3 license poses constraints in Android's user-space.
>
>This is the reason for the this change, to have at least the team options exposed through sysfs.

No, that is certainly not the reason. If libnl3 does not suit you, use a
different lib. It is in my opinion unacceptaptable to work around
certain licence issues by adding parallel sysfs api (not to mention that
sysfs is totally unsuitable for that).

You should use existing Netlink API. That's it.

>
>
>>
>>>
>>>Signed-off-by: Hamad Kadmany <hkadmany@codeaurora.org>
>>>---
>>> drivers/net/team/team.c | 282 ++++++++++++++++++++++++++++++++++++++++++++++--
>>> include/linux/if_team.h |   3 +
>>> 2 files changed, 278 insertions(+), 7 deletions(-)
>>>
>>>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>>>index 1222229..afd2f8f 100644
>>>--- a/drivers/net/team/team.c
>>>+++ b/drivers/net/team/team.c
>>>@@ -110,8 +110,198 @@ struct team_option_inst { /* One for each option instance */
>>> 	struct team_option_inst_info info;
>>> 	bool changed;
>>> 	bool removed;
>>>+	bool dev_attr_file_exist;
>>>+	struct device_attribute dev_attr; /* corresponding sysfs attribute */
>>> };
>>>
>>>+static struct attribute *team_sysfs_attrs[] = {
>>>+	NULL,
>>>+};
>>>+
>>>+static struct attribute_group team_sysfs_attr_group = {
>>>+	.attrs = team_sysfs_attrs,
>>>+};
>>>+
>>>+/* Device attributes (sysfs) */
>>>+
>>>+static ssize_t show_attribute(struct device *dev,
>>>+			      struct device_attribute *attr,
>>>+			      char *buf)
>>>+{
>>>+	struct team *team = dev_get_drvdata(dev);
>>>+	struct team_option_inst *opt_inst;
>>>+	ssize_t ret;
>>>+	struct team_option *option;
>>>+	struct team_gsetter_ctx ctx;
>>>+
>>>+	if (mutex_lock_interruptible(&team->lock))
>>>+		return -ERESTARTSYS;
>>>+
>>>+	opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>>+	option = opt_inst->option;
>>>+	if (!option->getter) {
>>>+		ret = -EOPNOTSUPP;
>>>+		netdev_err(team->dev,
>>>+			   "Option %s is write only\n", attr->attr.name);
>>>+		goto exit;
>>>+	}
>>>+
>>>+	ctx.info = &opt_inst->info;
>>>+	/* let the option getter do its job */
>>>+	ret = option->getter(team, &ctx);
>>>+	if (ret)
>>>+		goto exit;
>>>+
>>>+	/* translate option's output into sysfs output */
>>>+	switch (option->type) {
>>>+	case TEAM_OPTION_TYPE_U32:
>>>+		ret = scnprintf(buf, PAGE_SIZE, "%u\n", ctx.data.u32_val);
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_STRING:
>>>+		ret = scnprintf(buf, PAGE_SIZE, "%s\n", ctx.data.str_val);
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_BINARY:
>>>+		if (ctx.data.bin_val.len > PAGE_SIZE) {
>>>+			netdev_err(team->dev,
>>>+				   "Option output is too long (%d)\n",
>>>+				   ctx.data.bin_val.len);
>>>+			break;
>>>+		}
>>>+
>>>+		memcpy(buf, ctx.data.bin_val.ptr, ctx.data.bin_val.len);
>>>+		ret = ctx.data.bin_val.len;
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_BOOL:
>>>+		ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.bool_val);
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_S32:
>>>+		ret = scnprintf(buf, PAGE_SIZE, "%d\n", ctx.data.s32_val);
>>>+		break;
>>>+	default:
>>>+		BUG();
>>>+	}
>>>+
>>>+exit:
>>>+	mutex_unlock(&team->lock);
>>>+
>>>+	return ret;
>>>+}
>>>+
>>>+static int team_nl_send_event_options_get(struct team *team,
>>>+					  struct list_head *sel_opt_inst_list);
>>>+
>>>+static ssize_t set_attribute(struct device *dev,
>>>+			     struct device_attribute *attr,
>>>+			     const char *buf, size_t count)
>>>+{
>>>+	struct team_option_inst *opt_inst;
>>>+	struct team *team = dev_get_drvdata(dev);
>>>+	int err = 0;
>>>+	struct team_option *option = NULL;
>>>+	struct team_gsetter_ctx ctx;
>>>+
>>>+	LIST_HEAD(opt_inst_list);
>>>+
>>>+	if (mutex_lock_interruptible(&team->lock))
>>>+		return -ERESTARTSYS;
>>>+
>>>+	opt_inst = container_of(attr, struct team_option_inst, dev_attr);
>>>+	option = opt_inst->option;
>>>+	if (!option->setter) {
>>>+		netdev_err(team->dev,
>>>+			   "Option %s is read only\n", attr->attr.name);
>>>+		err = -EOPNOTSUPP;
>>>+		goto exit;
>>>+	}
>>>+
>>>+	ctx.info = &opt_inst->info;
>>>+
>>>+	/* translate sysfs input into option's input */
>>>+	switch (option->type) {
>>>+	case TEAM_OPTION_TYPE_U32:
>>>+		err = kstrtoint(buf, 0, &ctx.data.u32_val);
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_STRING:
>>>+		if (count > TEAM_STRING_MAX_LEN) {
>>>+			netdev_err(team->dev,
>>>+				   "Input buffer too long (%zu)\n", count);
>>>+			err = -EINVAL;
>>>+			break;
>>>+		}
>>>+		ctx.data.str_val = buf;
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_BINARY:
>>>+		ctx.data.bin_val.len = count;
>>>+		ctx.data.bin_val.ptr = buf;
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_BOOL:
>>>+		err = strtobool(buf, &ctx.data.bool_val);
>>>+		break;
>>>+	case TEAM_OPTION_TYPE_S32:
>>>+		err = kstrtoint(buf, 0, &ctx.data.s32_val);
>>>+		break;
>>>+	default:
>>>+		BUG();
>>>+	}
>>>+
>>>+	if (err) {
>>>+		netdev_err(team->dev, "Failed to translate input buffer\n");
>>>+		goto exit;
>>>+	}
>>>+
>>>+	/* let the option setter do its job */
>>>+	err = option->setter(team, &ctx);
>>>+	if (err)
>>>+		goto exit;
>>>+
>>>+	/* propagate option changed event */
>>>+	opt_inst->changed = true;
>>>+	list_add(&opt_inst->tmp_list, &opt_inst_list);
>>>+	err = team_nl_send_event_options_get(team, &opt_inst_list);
>>>+	if (err == -ESRCH) /* no listeners, not a real error */
>>>+		err = 0;
>>>+
>>>+exit:
>>>+	mutex_unlock(&team->lock);
>>>+
>>>+	if (!err)
>>>+		err = count;
>>>+	return err;
>>>+}
>>>+
>>>+/* create sysfs attribute for each option that is being registered */
>>>+static int __team_option_add_sysfs_attr(struct team *team,
>>>+					struct team_option_inst *opt_inst,
>>>+					bool create_sysfs_file)
>>>+{
>>>+	int err = 0;
>>>+	struct device_attribute *new_dev_attr = &opt_inst->dev_attr;
>>>+
>>>+	new_dev_attr->attr.name = opt_inst->option->name;
>>>+	new_dev_attr->attr.mode = S_IRUGO | S_IWUSR;
>>>+	new_dev_attr->show = show_attribute;
>>>+	new_dev_attr->store = set_attribute;
>>>+
>>>+	if (create_sysfs_file) {
>>>+		err = sysfs_create_file(&team->dev->dev.kobj,
>>>+					&new_dev_attr->attr);
>>>+		if (err)
>>>+			netdev_err(team->dev,
>>>+				   "Failed to create sysfs attribute %s\n",
>>>+				   new_dev_attr->attr.name);
>>>+	}
>>>+
>>>+	return err;
>>>+}
>>>+
>>>+static void __team_option_del_sysfs_attr(struct team *team,
>>>+					 struct team_option_inst *opt_inst)
>>>+{
>>>+	if (opt_inst->dev_attr_file_exist)
>>>+		sysfs_remove_file(&team->dev->dev.kobj,
>>>+				  &opt_inst->dev_attr.attr);
>>>+}
>>>+
>>> static struct team_option *__team_find_option(struct team *team,
>>> 					      const char *opt_name)
>>> {
>>>@@ -124,8 +314,10 @@ static struct team_option *__team_find_option(struct team *team,
>>> 	return NULL;
>>> }
>>>
>>>-static void __team_option_inst_del(struct team_option_inst *opt_inst)
>>>+static void __team_option_inst_del(struct team *team,
>>>+				   struct team_option_inst *opt_inst)
>>> {
>>>+	__team_option_del_sysfs_attr(team, opt_inst);
>>> 	list_del(&opt_inst->list);
>>> 	kfree(opt_inst);
>>> }
>>>@@ -137,7 +329,7 @@ static void __team_option_inst_del_option(struct team *team,
>>>
>>> 	list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>>> 		if (opt_inst->option == option)
>>>-			__team_option_inst_del(opt_inst);
>>>+			__team_option_inst_del(team, opt_inst);
>>> 	}
>>> }
>>>
>>>@@ -162,6 +354,7 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>>> 		opt_inst->info.array_index = i;
>>> 		opt_inst->changed = true;
>>> 		opt_inst->removed = false;
>>>+		opt_inst->dev_attr_file_exist = false;
>>> 		list_add_tail(&opt_inst->list, &team->option_inst_list);
>>> 		if (option->init) {
>>> 			err = option->init(team, &opt_inst->info);
>>>@@ -170,6 +363,20 @@ static int __team_option_inst_add(struct team *team, struct team_option *option,
>>> 		}
>>>
>>> 	}
>>>+
>>>+	/* add sysfs attribute. per-port and array options are skipped */
>>>+	if (!option->per_port && !option->array_size) {
>>>+		/* create the sysfs file only if our state allows it */
>>>+		bool create_sysfs_file = device_is_registered(&team->dev->dev);
>>>+
>>>+		err = __team_option_add_sysfs_attr(team, opt_inst,
>>>+						   create_sysfs_file);
>>>+		if (err)
>>>+			return err;
>>>+
>>>+		opt_inst->dev_attr_file_exist = true;
>>>+	}
>>>+
>>> 	return 0;
>>> }
>>>
>>>@@ -218,7 +425,7 @@ static void __team_option_inst_del_port(struct team *team,
>>> 	list_for_each_entry_safe(opt_inst, tmp, &team->option_inst_list, list) {
>>> 		if (opt_inst->option->per_port &&
>>> 		    opt_inst->info.port == port)
>>>-			__team_option_inst_del(opt_inst);
>>>+			__team_option_inst_del(team, opt_inst);
>>> 	}
>>> }
>>>
>>>@@ -337,6 +544,51 @@ static void __team_options_unregister(struct team *team,
>>>
>>> static void __team_options_change_check(struct team *team);
>>>
>>>+static void team_attr_grp_free(struct team *team)
>>>+{
>>>+	kfree(team->attr_grp.attrs);
>>>+}
>>>+
>>>+/* allocate attribute group for creating sysfs for team's own options */
>>>+static int team_attr_grp_alloc(struct team *team)
>>>+{
>>>+	struct attribute **attributes;
>>>+	struct team_option_inst *opt_inst;
>>>+	int num_attr = 0;
>>>+	struct team_option *option;
>>>+
>>>+	list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>>+		option = opt_inst->option;
>>>+		/* per-port and array options currently not supported as
>>>+		 * sysfs attributes
>>>+		 */
>>>+		if (option->per_port || option->array_size)
>>>+			continue;
>>>+
>>>+		num_attr++;
>>>+	}
>>>+
>>>+	/* +1 for having NULL as last item in the array */
>>>+	attributes = kzalloc((num_attr + 1) * sizeof(*attributes), GFP_KERNEL);
>>>+	if (!attributes)
>>>+		return -ENOMEM;
>>>+	team->attr_grp.attrs = attributes;
>>>+
>>>+	num_attr = 0;
>>>+	list_for_each_entry(opt_inst, &team->option_inst_list, list) {
>>>+		option = opt_inst->option;
>>>+		/* per-port and array options currently not supported as
>>>+		 * sysfs attributes
>>>+		 */
>>>+		if (option->per_port || option->array_size)
>>>+			continue;
>>>+
>>>+		attributes[num_attr++] = &opt_inst->dev_attr.attr;
>>>+	}
>>>+
>>>+	return 0;
>>>+}
>>>+
>>> int team_options_register(struct team *team,
>>> 			  const struct team_option *option,
>>> 			  size_t option_count)
>>>@@ -1380,15 +1632,28 @@ static int team_init(struct net_device *dev)
>>>
>>> 	INIT_LIST_HEAD(&team->option_list);
>>> 	INIT_LIST_HEAD(&team->option_inst_list);
>>>-	err = team_options_register(team, team_options, ARRAY_SIZE(team_options));
>>>+
>>>+	err = team_options_register(team, team_options,
>>>+				    ARRAY_SIZE(team_options));
>>> 	if (err)
>>> 		goto err_options_register;
>>> 	netif_carrier_off(dev);
>>>
>>> 	team_set_lockdep_class(dev);
>>>
>>>+	/* store team context, to be used by Device attributes getter/setter */
>>>+	dev_set_drvdata(&dev->dev, team);
>>>+
>>>+	/* allocate and register sysfs attributes for team's own options */
>>>+	err = team_attr_grp_alloc(team);
>>>+	if (err)
>>>+		goto err_grp_alloc;
>>>+	dev->sysfs_groups[0] = &team->attr_grp;
>>>+
>>> 	return 0;
>>>
>>>+err_grp_alloc:
>>>+	team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>>> err_options_register:
>>> 	team_queue_override_fini(team);
>>> err_team_queue_override_init:
>>>@@ -1407,9 +1672,15 @@ static void team_uninit(struct net_device *dev)
>>> 	list_for_each_entry_safe(port, tmp, &team->port_list, list)
>>> 		team_port_del(team, port->dev);
>>>
>>>+	sysfs_remove_group(&team->dev->dev.kobj, &team->attr_grp);
>>>+	team_attr_grp_free(team);
>>>+	/* set to dummy group to avoid double free by core */
>>>+	dev->sysfs_groups[0] = &team_sysfs_attr_group;
>>>+
>>> 	__team_change_mode(team, NULL); /* cleanup */
>>> 	__team_options_unregister(team, team_options, ARRAY_SIZE(team_options));
>>> 	team_queue_override_fini(team);
>>>+
>>> 	mutex_unlock(&team->lock);
>>> }
>>>
>>>@@ -2194,9 +2465,6 @@ static int team_nl_cmd_options_get(struct sk_buff *skb, struct genl_info *info)
>>> 	return err;
>>> }
>>>
>>>-static int team_nl_send_event_options_get(struct team *team,
>>>-					  struct list_head *sel_opt_inst_list);
>>>-
>>> static int team_nl_cmd_options_set(struct sk_buff *skb, struct genl_info *info)
>>> {
>>> 	struct team *team;
>>>diff --git a/include/linux/if_team.h b/include/linux/if_team.h
>>>index 25b8b15..2e9fb2a 100644
>>>--- a/include/linux/if_team.h
>>>+++ b/include/linux/if_team.h
>>>@@ -188,6 +188,9 @@ struct team {
>>> 	struct list_head option_list;
>>> 	struct list_head option_inst_list; /* list of option instances */
>>>
>>>+	/* attribute group for registering team's own options at init */
>>>+	struct attribute_group attr_grp;
>>>+
>>> 	const struct team_mode *mode;
>>> 	struct team_mode_ops ops;
>>> 	bool user_carrier_enabled;
>>>--
>>>1.8.5.2
>>>--
>>>Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
>>>The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>>>a Linux Foundation Collaborative Project
>>>
>>
>
>

^ permalink raw reply


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