* Re: [PATCH 1/1] iproute2: rsvp classifier support for multiple actions
From: Stephen Hemminger @ 2014-09-29 15:48 UTC (permalink / raw)
To: Jamal Hadi Salim; +Cc: john.fastabend, netdev
In-Reply-To: <1411922993-5828-1-git-send-email-jhs@emojatatu.com>
On Sun, 28 Sep 2014 12:49:53 -0400
Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
>
> Example setup:
>
> sudo tc qdisc del dev eth0 root handle 1:0 prio
> sudo tc qdisc add dev eth0 root handle 1:0 prio
>
> sudo tc filter add dev eth0 pref 10 proto ip parent 1:0 \
> rsvp session 10.0.0.1 ipproto icmp \
> classid 1:1 \
> action police rate 1kbit burst 90k pipe \
> action ok
>
> tc -s filter show dev eth0 parent 1:0
>
> filter protocol ip pref 10 rsvp
> filter protocol ip pref 10 rsvp fh 0x0001100a flowid 1:1 session
> 10.0.0.1 ipproto icmp
> action order 1: police 0x5 rate 1Kbit burst 23440b mtu 2Kb
> action pipe overhead 0b
> ref 1 bind 1
> Action statistics:
> Sent 98000 bytes 1000 pkt (dropped 0, overlimits 761 requeues 0)
> backlog 0b 0p requeues 0
>
> action order 2: gact action pass
> random type none pass val 0
> index 2 ref 1 bind 1 installed 60 sec used 3 sec
> Action statistics:
> Sent 74578 bytes 761 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Applied both rsvp patches
^ permalink raw reply
* Re: [PATCH] xen/xenbus: Use 'void' instead of 'int' for the return of xenbus_switch_state()
From: Konrad Rzeszutek Wilk @ 2014-09-29 15:40 UTC (permalink / raw)
To: David Vrabel
Cc: Chen Gang, ian.campbell, wei.liu2, boris.ostrovsky, bhelgaas,
jgross, yongjun_wei, mukesh.rathor, xen-devel,
netdev@vger.kernel.org, linux-pci, linux-scsi,
linux-kernel@vger.kernel.org
In-Reply-To: <542969E6.1010104@citrix.com>
On Mon, Sep 29, 2014 at 03:17:10PM +0100, David Vrabel wrote:
> On 29/09/14 15:02, Konrad Rzeszutek Wilk wrote:
> > On Sat, Sep 27, 2014 at 12:36:42AM +0800, Chen Gang wrote:
> >> When xenbus_switch_state() fails, it will call xenbus_switch_fatal()
> >
> > Only on the first depth, not on the subsequent ones (as in if
> > the first xenbus_switch_fail fails, it won't try to call
> > xenbus_switch_state again and again).
> >
> >> internally, so need not return any status value, then use 'void' instead
> >> of 'int' for xenbus_switch_state() and __xenbus_switch_state().
> >
> > When that switch occurs (to XenbusStateConnected) won't the watches
> > fire - meaning we MUST make sure that the watch functions - if they
> > use the xenbus_switch_state() they MUST not hold any locks - because
> > they could be executed once more?
> >
> > Oh wait, we don't have to worry about that right now as the callbacks
> > that pick up the messages from the XenBus are all gated on one mutex
> > anyhow.
> >
> > Hm, anyhow, I would add this extra piece of information to the patch:
> >
> >
> > diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
> > index c214daa..f7399fd 100644
> > --- a/drivers/xen/xen-pciback/xenbus.c
> > +++ b/drivers/xen/xen-pciback/xenbus.c
> > @@ -661,6 +661,12 @@ static void xen_pcibk_be_watch(struct xenbus_watch *watch,
> >
> > switch (xenbus_read_driver_state(pdev->xdev->nodename)) {
> > case XenbusStateInitWait:
> > + /*
> > + * xenbus_switch_state can call xenbus_switch_fatal which will
> > + * immediately set the state to XenbusStateClosing which
> > + * means if we were reading for it here we MUST drop any
> > + * locks so that we don't dead-lock.
> > + */
>
> Watches are asynchronous and serialised by the xenwatch thread. I can't
> see what deadlock you're talking about here. Particularly since the
> backend doesn't watch its own state node (it watches the frontend one).
>
> > xen_pcibk_setup_backend(pdev);
> > break;
> >
> >>
> >> Also need be sure that all callers which check the return value must let
> >> 'err' be 0.
> >
> > I am bit uncomfortable with that, that is due to:
> >
> >
> > .. snip..
> >> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> >> index 9c47b89..b5c3d47 100644
> >> --- a/drivers/net/xen-netback/xenbus.c
> >> +++ b/drivers/net/xen-netback/xenbus.c
> >> @@ -337,10 +337,7 @@ static int netback_probe(struct xenbus_device *dev,
> >> if (err)
> >> pr_debug("Error writing multi-queue-max-queues\n");
> >>
> >> - err = xenbus_switch_state(dev, XenbusStateInitWait);
> >> - if (err)
> >> - goto fail;
> >> -
> >> + xenbus_switch_state(dev, XenbusStateInitWait);
> >
> > Which if it fails it won't call:
> >
> > 354 fail:
> > 355 pr_debug("failed\n");
> > 356 netback_remove(dev);
> > 357 return err;
> >
> >
> > And since there is no watch on the backend state to go in Closing it won't
> > ever call those and we leak memory.
>
> It's not leaking the memory. All resources will be recovered when the
> device is removed.
I presume you mean when the XenBus entries are torn down? It does look
like it would call the .remove functionality. That should take care of that.
In which case we can just remove all of the 'netback_remove()' and also
remove some of the labels.
>
> > The same is for xen-blkback mechanism in the probe function.
>
> David
>
^ permalink raw reply
* Re: [PATCH net-next v3] bonding: make global bonding stats more reliable
From: Andy Gospodarek @ 2014-09-29 15:27 UTC (permalink / raw)
To: Ding Tianhong; +Cc: netdev, j.vosburgh, vfalico, nikolay
In-Reply-To: <5428D781.4040700@huawei.com>
On Mon, Sep 29, 2014 at 11:52:33AM +0800, Ding Tianhong wrote:
> On 2014/9/26 9:12, Andy Gospodarek wrote:
> > As the code stands today, bonding stats are based simply on the stats
> > from the member interfaces. If a member was to be removed from a bond,
> > the stats would instantly drop. This would be confusing to an admin
> > would would suddonly see interface stats drop while traffic is still
> > flowing.
> >
> > In addition to preventing the stats drops mentioned above, new members
> > will now be added to the bond and only traffic received after the member
> > was added to the bond will be counted as part of bonding stats.
> >
> > v2: Changes suggested by Nik to properly allocate/free stats memory.
> > v3: Properly destroy workqueue and fix netlink configuration path.
> >
> > Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com>
> > ---
> > drivers/net/bonding/bond_main.c | 84 +++++++++++++++++++++++++-------------
> > drivers/net/bonding/bond_netlink.c | 13 +++++-
> > drivers/net/bonding/bonding.h | 3 ++
> > 3 files changed, 71 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > index 5390475..10c2dc3 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -1147,17 +1147,26 @@ static struct slave *bond_alloc_slave(struct bonding *bond)
> >
> > slave = kzalloc(sizeof(struct slave), GFP_KERNEL);
> > if (!slave)
> > - return NULL;
> > + goto slave_fail;
> > +
> > + slave->slave_stats = kzalloc(sizeof(struct rtnl_link_stats64),
> > + GFP_KERNEL);
> > + if (!slave->slave_stats)
> > + goto slave_stats_fail;
> >
> > if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> > SLAVE_AD_INFO(slave) = kzalloc(sizeof(struct ad_slave_info),
> > GFP_KERNEL);
> > - if (!SLAVE_AD_INFO(slave)) {
> > - kfree(slave);
> > - return NULL;
> > - }
> > + if (!SLAVE_AD_INFO(slave))
> > + goto slave_ad_fail;
> > }
> > return slave;
> > +slave_ad_fail:
> > + kfree(slave->slave_stats);
> > +slave_stats_fail:
> > + kfree(slave);
> > +slave_fail:
> > + return NULL;
> > }
> >
> > static void bond_free_slave(struct slave *slave)
> > @@ -1167,6 +1176,7 @@ static void bond_free_slave(struct slave *slave)
> > if (BOND_MODE(bond) == BOND_MODE_8023AD)
> > kfree(SLAVE_AD_INFO(slave));
> >
> > + kfree(slave->slave_stats);
> > kfree(slave);
> > }
> >
> > @@ -1344,6 +1354,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> > }
> >
> > slave_dev->priv_flags |= IFF_BONDING;
> > + /* initialize slave stats */
> > + dev_get_stats(new_slave->dev, new_slave->slave_stats);
> >
> > if (bond_is_lb(bond)) {
> > /* bond_alb_init_slave() must be called before all other stages since
> > @@ -3085,38 +3097,43 @@ static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
> > struct list_head *iter;
> > struct slave *slave;
> >
> > - memset(stats, 0, sizeof(*stats));
> > + memcpy(stats, bond->bond_stats, sizeof(*stats));
> >
> > bond_for_each_slave(bond, slave, iter) {
> > const struct rtnl_link_stats64 *sstats =
> > dev_get_stats(slave->dev, &temp);
> > + struct rtnl_link_stats64 *pstats = slave->slave_stats;
> > +
> > + stats->rx_packets += sstats->rx_packets - pstats->rx_packets;
> > + stats->rx_bytes += sstats->rx_bytes - pstats->rx_bytes;
> > + stats->rx_errors += sstats->rx_errors - pstats->rx_errors;
> > + stats->rx_dropped += sstats->rx_dropped - pstats->rx_dropped;
> >
> > - stats->rx_packets += sstats->rx_packets;
> > - stats->rx_bytes += sstats->rx_bytes;
> > - stats->rx_errors += sstats->rx_errors;
> > - stats->rx_dropped += sstats->rx_dropped;
> > + stats->tx_packets += sstats->tx_packets - pstats->tx_packets;;
> > + stats->tx_bytes += sstats->tx_bytes - pstats->tx_bytes;
> > + stats->tx_errors += sstats->tx_errors - pstats->tx_errors;
> > + stats->tx_dropped += sstats->tx_dropped - pstats->tx_dropped;
> >
> > - stats->tx_packets += sstats->tx_packets;
> > - stats->tx_bytes += sstats->tx_bytes;
> > - stats->tx_errors += sstats->tx_errors;
> > - stats->tx_dropped += sstats->tx_dropped;
> > + stats->multicast += sstats->multicast - pstats->multicast;
> > + stats->collisions += sstats->collisions - pstats->collisions;
> >
> > - stats->multicast += sstats->multicast;
> > - stats->collisions += sstats->collisions;
> > + stats->rx_length_errors += sstats->rx_length_errors - pstats->rx_length_errors;
> > + stats->rx_over_errors += sstats->rx_over_errors - pstats->rx_over_errors;
> > + stats->rx_crc_errors += sstats->rx_crc_errors - pstats->rx_crc_errors;
> > + stats->rx_frame_errors += sstats->rx_frame_errors - pstats->rx_frame_errors;
> > + stats->rx_fifo_errors += sstats->rx_fifo_errors - pstats->rx_fifo_errors;
> > + stats->rx_missed_errors += sstats->rx_missed_errors - pstats->rx_missed_errors;
> >
> > - stats->rx_length_errors += sstats->rx_length_errors;
> > - stats->rx_over_errors += sstats->rx_over_errors;
> > - stats->rx_crc_errors += sstats->rx_crc_errors;
> > - stats->rx_frame_errors += sstats->rx_frame_errors;
> > - stats->rx_fifo_errors += sstats->rx_fifo_errors;
> > - stats->rx_missed_errors += sstats->rx_missed_errors;
> > + stats->tx_aborted_errors += sstats->tx_aborted_errors - pstats->tx_aborted_errors;
> > + stats->tx_carrier_errors += sstats->tx_carrier_errors - pstats->tx_carrier_errors;
> > + stats->tx_fifo_errors += sstats->tx_fifo_errors - pstats->tx_fifo_errors;
> > + stats->tx_heartbeat_errors += sstats->tx_heartbeat_errors - pstats->tx_heartbeat_errors;
> > + stats->tx_window_errors += sstats->tx_window_errors - pstats->tx_window_errors;
> >
> > - stats->tx_aborted_errors += sstats->tx_aborted_errors;
> > - stats->tx_carrier_errors += sstats->tx_carrier_errors;
> > - stats->tx_fifo_errors += sstats->tx_fifo_errors;
> > - stats->tx_heartbeat_errors += sstats->tx_heartbeat_errors;
> > - stats->tx_window_errors += sstats->tx_window_errors;
> > + /* save off the slave stats for the next run */
> > + memcpy(pstats, sstats, sizeof(*sstats));
> > }
> > + memcpy(bond->bond_stats, stats, sizeof(*stats));
> >
> > return stats;
> > }
> > @@ -3790,6 +3807,9 @@ static void bond_destructor(struct net_device *bond_dev)
> > struct bonding *bond = netdev_priv(bond_dev);
> > if (bond->wq)
> > destroy_workqueue(bond->wq);
> > + if (bond->bond_stats)
> > + kfree(bond->bond_stats);
> > +
> > free_netdev(bond_dev);
> > }
> >
> > @@ -4274,7 +4294,8 @@ unsigned int bond_get_num_tx_queues(void)
> > int bond_create(struct net *net, const char *name)
> > {
> > struct net_device *bond_dev;
> > - int res;
> > + struct bonding *bond;
> > + int res = -ENOMEM;
> >
> > rtnl_lock();
> >
> > @@ -4286,8 +4307,15 @@ int bond_create(struct net *net, const char *name)
> > rtnl_unlock();
> > return -ENOMEM;
> > }
> > + /* alloc persistent stats for the bond */
> > + bond = netdev_priv(bond_dev);
> > + bond->bond_stats = kzalloc(sizeof(struct rtnl_link_stats64),
> > + GFP_KERNEL);
> > + if (!bond->bond_stats)
> > + printk(KERN_CRIT "Stats not available!\n");
> >
> hi Andy:
>
> why not return here, otherwise terrible things will happen, later other code will access this point directly.
>
Ding, thanks for the review.
I'm actually going to drop the kzalloc() in the next series and just add
the structs directly to struct bonding and struct slave instead of the
pointers.
I had someone else make one more suggestion to update bond_stats when a
slave is removed, so I will also make that change and test before the
next version.
> Ding
>
> > dev_net_set(bond_dev, net);
> > +
> > bond_dev->rtnl_link_ops = &bond_link_ops;
> >
> > res = register_netdevice(bond_dev);
> > diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> > index c13d83e..2fdcbe2 100644
> > --- a/drivers/net/bonding/bond_netlink.c
> > +++ b/drivers/net/bonding/bond_netlink.c
> > @@ -381,10 +381,21 @@ static int bond_newlink(struct net *src_net, struct net_device *bond_dev,
> > struct nlattr *tb[], struct nlattr *data[])
> > {
> > int err;
> > + struct bonding *bond = netdev_priv(bond_dev);
> > +
> > + /* alloc persistent stats for the bond if not already allocated */
> > + if (!bond->bond_stats) {
> > + bond->bond_stats = kzalloc(sizeof(struct rtnl_link_stats64),
> > + GFP_KERNEL);
> > + if (!bond->bond_stats)
> > + return -ENOMEM;
> > + }
> >
> > err = bond_changelink(bond_dev, tb, data);
> > - if (err < 0)
> > + if (err < 0) {
> > + kfree(bond->bond_stats);
> > return err;
> > + }
> >
> > return register_netdevice(bond_dev);
> > }
> > diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> > index 6140bf0..fe25265 100644
> > --- a/drivers/net/bonding/bonding.h
> > +++ b/drivers/net/bonding/bonding.h
> > @@ -24,6 +24,7 @@
> > #include <linux/inetdevice.h>
> > #include <linux/etherdevice.h>
> > #include <linux/reciprocal_div.h>
> > +#include <linux/if_link.h>
> >
> > #include "bond_3ad.h"
> > #include "bond_alb.h"
> > @@ -175,6 +176,7 @@ struct slave {
> > struct netpoll *np;
> > #endif
> > struct kobject kobj;
> > + struct rtnl_link_stats64 *slave_stats;
> > };
> >
> > /*
> > @@ -224,6 +226,7 @@ struct bonding {
> > /* debugging support via debugfs */
> > struct dentry *debug_dir;
> > #endif /* CONFIG_DEBUG_FS */
> > + struct rtnl_link_stats64 *bond_stats;
> > };
> >
> > #define bond_slave_get_rcu(dev) \
> >
>
>
^ permalink raw reply
* Re: [PATCH] staging: r8821ae: Remove driver from staging
From: Larry Finger @ 2014-09-29 14:45 UTC (permalink / raw)
To: Greg KH; +Cc: devel, netdev
In-Reply-To: <20140929061744.GA25093@kroah.com>
On 09/29/2014 01:17 AM, Greg KH wrote:
> On Sun, Sep 28, 2014 at 08:39:51PM -0500, Larry Finger wrote:
>> A new version of this driver has been merged into the wireless-testing tree
>> as commit 21e4b0726dc671c423e2dc9a85364716219c4502. It is both possible and
>> desirable to delete the staging version to avoid build errors in linux-next.
>>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> ---
>>
>> Greg,
>>
>> This removal patch was generated after commit 368c75b98bdfdfad54e7f165016819ef344e3587.
>> It should apply to your tree unless further modifications have been made to
>> the staging driver.
>
> Wonderful!
>
> So, this should be done for 3.18-rc1, right? Or does wireless-testing
> take longer to get to Linus than that?
Yes, it should be for 3.18-rc1. Sorry that I did not make that more clear.
It is also likely that I will be sending a similar delete patch for r8192ee.
John has the patch to add a new version to wireless-testing, but has not yet
merged it.
Larry
^ permalink raw reply
* Re: [PATCH v5 09/10] ARM: dts: berlin: add the Ethernet node
From: Sebastian Hesselbarth @ 2014-09-29 14:34 UTC (permalink / raw)
To: Sergei Shtylyov, Antoine Tenart
Cc: thomas.petazzoni, zmxu, devicetree, netdev, linux-kernel,
alexandre.belloni, jszhang, linux-arm-kernel
In-Reply-To: <5425A178.4050501@cogentembedded.com>
On 09/26/2014 07:25 PM, Sergei Shtylyov wrote:
> On 09/26/2014 06:33 PM, Antoine Tenart wrote:
>> This patch adds the Ethernet node, enabling the network unit on Berlin
>> BG2Q SoCs.
>
>> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> arch/arm/boot/dts/berlin2q.dtsi | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>
>> diff --git a/arch/arm/boot/dts/berlin2q.dtsi
>> b/arch/arm/boot/dts/berlin2q.dtsi
>> index 902eddb19cd8..d442b22fd1ea 100644
>> --- a/arch/arm/boot/dts/berlin2q.dtsi
>> +++ b/arch/arm/boot/dts/berlin2q.dtsi
>> @@ -114,6 +114,23 @@
>> #interrupt-cells = <3>;
>> };
>>
>> + eth0: ethernet@b90000 {
>> + compatible = "marvell,pxa168-eth";
>> + reg = <0xb90000 0x10000>;
>> + clocks = <&chip CLKID_GETH0>;
>> + interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
>> + /* set by bootloader */
>> + local-mac-address = [00 00 00 00 00 00];
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + phy-handle = <ðphy0>;
>> + status = "disabled";
>> +
>> + ethphy0: ethernet-phy@0 {
>> + reg = <0>;
>> + };
>> + };
>> +
>
> Hm, is the PHY internal to the Ethernet controller?
Sergei,
at least for BG2 and BG2CD the PHY is internal to the SoC. Also,
MDIO bus is part of the ethernet IP, so placing the PHY inside
the combined Ethernet/MDIO IP node is sane.
Sebastian
^ permalink raw reply
* Re: [PATCH] xen/xenbus: Use 'void' instead of 'int' for the return of xenbus_switch_state()
From: David Vrabel @ 2014-09-29 14:17 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, Chen Gang
Cc: ian.campbell, wei.liu2, boris.ostrovsky, bhelgaas, jgross,
yongjun_wei, mukesh.rathor, xen-devel, netdev@vger.kernel.org,
linux-pci, linux-scsi, linux-kernel@vger.kernel.org
In-Reply-To: <20140929140229.GA28279@laptop.dumpdata.com>
On 29/09/14 15:02, Konrad Rzeszutek Wilk wrote:
> On Sat, Sep 27, 2014 at 12:36:42AM +0800, Chen Gang wrote:
>> When xenbus_switch_state() fails, it will call xenbus_switch_fatal()
>
> Only on the first depth, not on the subsequent ones (as in if
> the first xenbus_switch_fail fails, it won't try to call
> xenbus_switch_state again and again).
>
>> internally, so need not return any status value, then use 'void' instead
>> of 'int' for xenbus_switch_state() and __xenbus_switch_state().
>
> When that switch occurs (to XenbusStateConnected) won't the watches
> fire - meaning we MUST make sure that the watch functions - if they
> use the xenbus_switch_state() they MUST not hold any locks - because
> they could be executed once more?
>
> Oh wait, we don't have to worry about that right now as the callbacks
> that pick up the messages from the XenBus are all gated on one mutex
> anyhow.
>
> Hm, anyhow, I would add this extra piece of information to the patch:
>
>
> diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
> index c214daa..f7399fd 100644
> --- a/drivers/xen/xen-pciback/xenbus.c
> +++ b/drivers/xen/xen-pciback/xenbus.c
> @@ -661,6 +661,12 @@ static void xen_pcibk_be_watch(struct xenbus_watch *watch,
>
> switch (xenbus_read_driver_state(pdev->xdev->nodename)) {
> case XenbusStateInitWait:
> + /*
> + * xenbus_switch_state can call xenbus_switch_fatal which will
> + * immediately set the state to XenbusStateClosing which
> + * means if we were reading for it here we MUST drop any
> + * locks so that we don't dead-lock.
> + */
Watches are asynchronous and serialised by the xenwatch thread. I can't
see what deadlock you're talking about here. Particularly since the
backend doesn't watch its own state node (it watches the frontend one).
> xen_pcibk_setup_backend(pdev);
> break;
>
>>
>> Also need be sure that all callers which check the return value must let
>> 'err' be 0.
>
> I am bit uncomfortable with that, that is due to:
>
>
> .. snip..
>> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
>> index 9c47b89..b5c3d47 100644
>> --- a/drivers/net/xen-netback/xenbus.c
>> +++ b/drivers/net/xen-netback/xenbus.c
>> @@ -337,10 +337,7 @@ static int netback_probe(struct xenbus_device *dev,
>> if (err)
>> pr_debug("Error writing multi-queue-max-queues\n");
>>
>> - err = xenbus_switch_state(dev, XenbusStateInitWait);
>> - if (err)
>> - goto fail;
>> -
>> + xenbus_switch_state(dev, XenbusStateInitWait);
>
> Which if it fails it won't call:
>
> 354 fail:
> 355 pr_debug("failed\n");
> 356 netback_remove(dev);
> 357 return err;
>
>
> And since there is no watch on the backend state to go in Closing it won't
> ever call those and we leak memory.
It's not leaking the memory. All resources will be recovered when the
device is removed.
> The same is for xen-blkback mechanism in the probe function.
David
^ permalink raw reply
* Re: [Xen-devel] [PATCH] xen/xenbus: Use 'void' instead of 'int' for the return of xenbus_switch_state()
From: Konrad Rzeszutek Wilk @ 2014-09-29 14:03 UTC (permalink / raw)
To: David Vrabel
Cc: Chen Gang, ian.campbell, wei.liu2, boris.ostrovsky, bhelgaas,
jgross, yongjun_wei, mukesh.rathor, xen-devel, linux-pci,
linux-kernel@vger.kernel.org, linux-scsi, netdev@vger.kernel.org
In-Reply-To: <5425AB57.8040804@citrix.com>
On Fri, Sep 26, 2014 at 07:07:19PM +0100, David Vrabel wrote:
> On 26/09/14 17:36, Chen Gang wrote:
> > When xenbus_switch_state() fails, it will call xenbus_switch_fatal()
> > internally, so need not return any status value, then use 'void' instead
> > of 'int' for xenbus_switch_state() and __xenbus_switch_state().
> >
> > Also need be sure that all callers which check the return value must let
> > 'err' be 0.
>
> I've rewritten the commit message as:
>
> xen/xenbus: don't return errors from xenbus_switch_state()
>
> Most users of xenbus_switch_state() weren't handling the failure of
> xenbus_switch_state() correctly. They either called
> xenbus_dev_fatal() (which xenbus_switch_state() has effectively
> already tried to do), or ignored errors.
>
> xenbus_switch_state() may fail because:
>
> a) The device is being unplugged by the toolstack. The device will
> shortly be removed and the error does not need to be handled.
>
> b) Xenstore is broken. There isn't much the driver can do in this
> case since xenstore is required to signal failure to the toolstack.
>
> So, don't report any errors from xenbus_switch_state() which removes
> some unnecessary error handling in some of the drivers.
>
> I'd appreciate a review from some of the other front/backend driver
> maintainers on whether this is sensible reasoning.
Done that but I am not too keen about this patch - as I think it will
cause memory leaks on failure paths.
>
> David
>
> >
> > And also need change the related comments for xenbus_switch_state().
> >
> > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> > ---
> > drivers/block/xen-blkback/xenbus.c | 9 ++-------
> > drivers/net/xen-netback/xenbus.c | 5 +----
> > drivers/pci/xen-pcifront.c | 15 ++++++---------
> > drivers/xen/xen-pciback/xenbus.c | 21 ++++-----------------
> > drivers/xen/xen-scsiback.c | 5 +----
> > drivers/xen/xenbus/xenbus_client.c | 16 ++++++++--------
> > include/xen/xenbus.h | 3 ++-
> > 7 files changed, 24 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
> > index 3a8b810..fdcc584 100644
> > --- a/drivers/block/xen-blkback/xenbus.c
> > +++ b/drivers/block/xen-blkback/xenbus.c
> > @@ -587,9 +587,7 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
> > if (err)
> > goto fail;
> >
> > - err = xenbus_switch_state(dev, XenbusStateInitWait);
> > - if (err)
> > - goto fail;
> > + xenbus_switch_state(dev, XenbusStateInitWait);
> >
> > return 0;
> >
> > @@ -837,10 +835,7 @@ again:
> > if (err)
> > xenbus_dev_fatal(dev, err, "ending transaction");
> >
> > - err = xenbus_switch_state(dev, XenbusStateConnected);
> > - if (err)
> > - xenbus_dev_fatal(dev, err, "%s: switching to Connected state",
> > - dev->nodename);
> > + xenbus_switch_state(dev, XenbusStateConnected);
> >
> > return;
> > abort:
> > diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> > index 9c47b89..b5c3d47 100644
> > --- a/drivers/net/xen-netback/xenbus.c
> > +++ b/drivers/net/xen-netback/xenbus.c
> > @@ -337,10 +337,7 @@ static int netback_probe(struct xenbus_device *dev,
> > if (err)
> > pr_debug("Error writing multi-queue-max-queues\n");
> >
> > - err = xenbus_switch_state(dev, XenbusStateInitWait);
> > - if (err)
> > - goto fail;
> > -
> > + xenbus_switch_state(dev, XenbusStateInitWait);
> > be->state = XenbusStateInitWait;
> >
> > /* This kicks hotplug scripts, so do it immediately. */
> > diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> > index 53df39a..c1d73b7 100644
> > --- a/drivers/pci/xen-pcifront.c
> > +++ b/drivers/pci/xen-pcifront.c
> > @@ -901,18 +901,16 @@ static int pcifront_try_connect(struct pcifront_device *pdev)
> > }
> > }
> >
> > - err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
> > -
> > + xenbus_switch_state(pdev->xdev, XenbusStateConnected);
> > + return 0;
> > out:
> > return err;
> > }
> >
> > static int pcifront_try_disconnect(struct pcifront_device *pdev)
> > {
> > - int err = 0;
> > enum xenbus_state prev_state;
> >
> > -
> > prev_state = xenbus_read_driver_state(pdev->xdev->nodename);
> >
> > if (prev_state >= XenbusStateClosing)
> > @@ -923,11 +921,10 @@ static int pcifront_try_disconnect(struct pcifront_device *pdev)
> > pcifront_disconnect(pdev);
> > }
> >
> > - err = xenbus_switch_state(pdev->xdev, XenbusStateClosed);
> > + xenbus_switch_state(pdev->xdev, XenbusStateClosed);
> >
> > out:
> > -
> > - return err;
> > + return 0;
> > }
> >
> > static int pcifront_attach_devices(struct pcifront_device *pdev)
> > @@ -1060,8 +1057,8 @@ static int pcifront_detach_devices(struct pcifront_device *pdev)
> > domain, bus, slot, func);
> > }
> >
> > - err = xenbus_switch_state(pdev->xdev, XenbusStateReconfiguring);
> > -
> > + xenbus_switch_state(pdev->xdev, XenbusStateReconfiguring);
> > + return 0;
> > out:
> > return err;
> > }
> > diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
> > index c214daa..630a215 100644
> > --- a/drivers/xen/xen-pciback/xenbus.c
> > +++ b/drivers/xen/xen-pciback/xenbus.c
> > @@ -184,10 +184,7 @@ static int xen_pcibk_attach(struct xen_pcibk_device *pdev)
> >
> > dev_dbg(&pdev->xdev->dev, "Connecting...\n");
> >
> > - err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
> > - if (err)
> > - xenbus_dev_fatal(pdev->xdev, err,
> > - "Error switching to connected state!");
> > + xenbus_switch_state(pdev->xdev, XenbusStateConnected);
> >
> > dev_dbg(&pdev->xdev->dev, "Connected? %d\n", err);
> > out:
> > @@ -500,12 +497,7 @@ static int xen_pcibk_reconfigure(struct xen_pcibk_device *pdev)
> > }
> > }
> >
> > - err = xenbus_switch_state(pdev->xdev, XenbusStateReconfigured);
> > - if (err) {
> > - xenbus_dev_fatal(pdev->xdev, err,
> > - "Error switching to reconfigured state!");
> > - goto out;
> > - }
> > + xenbus_switch_state(pdev->xdev, XenbusStateReconfigured);
> >
> > out:
> > mutex_unlock(&pdev->dev_lock);
> > @@ -640,10 +632,7 @@ static int xen_pcibk_setup_backend(struct xen_pcibk_device *pdev)
> > goto out;
> > }
> >
> > - err = xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
> > - if (err)
> > - xenbus_dev_fatal(pdev->xdev, err,
> > - "Error switching to initialised state!");
> > + xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
> >
> > out:
> > mutex_unlock(&pdev->dev_lock);
> > @@ -683,9 +672,7 @@ static int xen_pcibk_xenbus_probe(struct xenbus_device *dev,
> > }
> >
> > /* wait for xend to configure us */
> > - err = xenbus_switch_state(dev, XenbusStateInitWait);
> > - if (err)
> > - goto out;
> > + xenbus_switch_state(dev, XenbusStateInitWait);
> >
> > /* watch the backend node for backend configuration information */
> > err = xenbus_watch_path(dev, dev->nodename, &pdev->be_watch,
> > diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
> > index ad11258..847bc9c 100644
> > --- a/drivers/xen/xen-scsiback.c
> > +++ b/drivers/xen/xen-scsiback.c
> > @@ -1225,10 +1225,7 @@ static int scsiback_probe(struct xenbus_device *dev,
> > if (err)
> > xenbus_dev_error(dev, err, "writing feature-sg-grant");
> >
> > - err = xenbus_switch_state(dev, XenbusStateInitWait);
> > - if (err)
> > - goto fail;
> > -
> > + xenbus_switch_state(dev, XenbusStateInitWait);
> > return 0;
> >
> > fail:
> > diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> > index ca74410..e2bcd1d 100644
> > --- a/drivers/xen/xenbus/xenbus_client.c
> > +++ b/drivers/xen/xenbus/xenbus_client.c
> > @@ -166,7 +166,7 @@ EXPORT_SYMBOL_GPL(xenbus_watch_pathfmt);
> > static void xenbus_switch_fatal(struct xenbus_device *, int, int,
> > const char *, ...);
> >
> > -static int
> > +static void
> > __xenbus_switch_state(struct xenbus_device *dev,
> > enum xenbus_state state, int depth)
> > {
> > @@ -188,7 +188,7 @@ __xenbus_switch_state(struct xenbus_device *dev,
> > int err, abort;
> >
> > if (state == dev->state)
> > - return 0;
> > + return;
> >
> > again:
> > abort = 1;
> > @@ -196,7 +196,7 @@ again:
> > err = xenbus_transaction_start(&xbt);
> > if (err) {
> > xenbus_switch_fatal(dev, depth, err, "starting transaction");
> > - return 0;
> > + return;
> > }
> >
> > err = xenbus_scanf(xbt, dev->nodename, "state", "%d", ¤t_state);
> > @@ -219,7 +219,7 @@ abort:
> > } else
> > dev->state = state;
> >
> > - return 0;
> > + return;
> > }
> >
> > /**
> > @@ -228,12 +228,12 @@ abort:
> > * @state: new state
> > *
> > * Advertise in the store a change of the given driver to the given new_state.
> > - * Return 0 on success, or -errno on error. On error, the device will switch
> > - * to XenbusStateClosing, and the error will be saved in the store.
> > + * When failure occurs, it will call xenbus_switch_fatal() internally, so need
> > + * not return value to notify upper caller.
> > */
> > -int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state state)
> > +void xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state state)
> > {
> > - return __xenbus_switch_state(dev, state, 0);
> > + __xenbus_switch_state(dev, state, 0);
> > }
> >
> > EXPORT_SYMBOL_GPL(xenbus_switch_state);
> > diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> > index 0324c6d..587c53d 100644
> > --- a/include/xen/xenbus.h
> > +++ b/include/xen/xenbus.h
> > @@ -195,7 +195,8 @@ int xenbus_watch_pathfmt(struct xenbus_device *dev, struct xenbus_watch *watch,
> > const char **, unsigned int),
> > const char *pathfmt, ...);
> >
> > -int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state new_state);
> > +void xenbus_switch_state(struct xenbus_device *dev,
> > + enum xenbus_state new_state);
> > int xenbus_grant_ring(struct xenbus_device *dev, unsigned long ring_mfn);
> > int xenbus_map_ring_valloc(struct xenbus_device *dev,
> > int gnt_ref, void **vaddr);
> >
>
^ permalink raw reply
* Re: [PATCH] xen/xenbus: Use 'void' instead of 'int' for the return of xenbus_switch_state()
From: Konrad Rzeszutek Wilk @ 2014-09-29 14:02 UTC (permalink / raw)
To: Chen Gang
Cc: David Vrabel, ian.campbell, wei.liu2, boris.ostrovsky, bhelgaas,
jgross, yongjun_wei, mukesh.rathor, xen-devel,
netdev@vger.kernel.org, linux-pci, linux-scsi,
linux-kernel@vger.kernel.org
In-Reply-To: <5425961A.5000604@gmail.com>
On Sat, Sep 27, 2014 at 12:36:42AM +0800, Chen Gang wrote:
> When xenbus_switch_state() fails, it will call xenbus_switch_fatal()
Only on the first depth, not on the subsequent ones (as in if
the first xenbus_switch_fail fails, it won't try to call
xenbus_switch_state again and again).
> internally, so need not return any status value, then use 'void' instead
> of 'int' for xenbus_switch_state() and __xenbus_switch_state().
When that switch occurs (to XenbusStateConnected) won't the watches
fire - meaning we MUST make sure that the watch functions - if they
use the xenbus_switch_state() they MUST not hold any locks - because
they could be executed once more?
Oh wait, we don't have to worry about that right now as the callbacks
that pick up the messages from the XenBus are all gated on one mutex
anyhow.
Hm, anyhow, I would add this extra piece of information to the patch:
diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
index c214daa..f7399fd 100644
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -661,6 +661,12 @@ static void xen_pcibk_be_watch(struct xenbus_watch *watch,
switch (xenbus_read_driver_state(pdev->xdev->nodename)) {
case XenbusStateInitWait:
+ /*
+ * xenbus_switch_state can call xenbus_switch_fatal which will
+ * immediately set the state to XenbusStateClosing which
+ * means if we were reading for it here we MUST drop any
+ * locks so that we don't dead-lock.
+ */
xen_pcibk_setup_backend(pdev);
break;
>
> Also need be sure that all callers which check the return value must let
> 'err' be 0.
I am bit uncomfortable with that, that is due to:
.. snip..
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index 9c47b89..b5c3d47 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -337,10 +337,7 @@ static int netback_probe(struct xenbus_device *dev,
> if (err)
> pr_debug("Error writing multi-queue-max-queues\n");
>
> - err = xenbus_switch_state(dev, XenbusStateInitWait);
> - if (err)
> - goto fail;
> -
> + xenbus_switch_state(dev, XenbusStateInitWait);
Which if it fails it won't call:
354 fail:
355 pr_debug("failed\n");
356 netback_remove(dev);
357 return err;
And since there is no watch on the backend state to go in Closing it won't
ever call those and we leak memory.
The same is for xen-blkback mechanism in the probe function.
> be->state = XenbusStateInitWait;
>
> /* This kicks hotplug scripts, so do it immediately. */
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 53df39a..c1d73b7 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -901,18 +901,16 @@ static int pcifront_try_connect(struct pcifront_device *pdev)
> }
> }
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
> -
> + xenbus_switch_state(pdev->xdev, XenbusStateConnected);
> + return 0;
> out:
> return err;
> }
>
> static int pcifront_try_disconnect(struct pcifront_device *pdev)
> {
> - int err = 0;
> enum xenbus_state prev_state;
>
> -
> prev_state = xenbus_read_driver_state(pdev->xdev->nodename);
>
> if (prev_state >= XenbusStateClosing)
> @@ -923,11 +921,10 @@ static int pcifront_try_disconnect(struct pcifront_device *pdev)
> pcifront_disconnect(pdev);
> }
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateClosed);
> + xenbus_switch_state(pdev->xdev, XenbusStateClosed);
>
> out:
> -
> - return err;
> + return 0;
> }
>
> static int pcifront_attach_devices(struct pcifront_device *pdev)
> @@ -1060,8 +1057,8 @@ static int pcifront_detach_devices(struct pcifront_device *pdev)
> domain, bus, slot, func);
> }
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateReconfiguring);
> -
> + xenbus_switch_state(pdev->xdev, XenbusStateReconfiguring);
> + return 0;
> out:
> return err;
> }
> diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
> index c214daa..630a215 100644
The xen-pcifront are OK, this one:
> --- a/drivers/xen/xen-pciback/xenbus.c
> +++ b/drivers/xen/xen-pciback/xenbus.c
> @@ -184,10 +184,7 @@ static int xen_pcibk_attach(struct xen_pcibk_device *pdev)
>
> dev_dbg(&pdev->xdev->dev, "Connecting...\n");
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
> - if (err)
> - xenbus_dev_fatal(pdev->xdev, err,
> - "Error switching to connected state!");
> + xenbus_switch_state(pdev->xdev, XenbusStateConnected);
This one is OK
>
> dev_dbg(&pdev->xdev->dev, "Connected? %d\n", err);
> out:
> @@ -500,12 +497,7 @@ static int xen_pcibk_reconfigure(struct xen_pcibk_device *pdev)
> }
> }
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateReconfigured);
> - if (err) {
> - xenbus_dev_fatal(pdev->xdev, err,
> - "Error switching to reconfigured state!");
> - goto out;
> - }
> + xenbus_switch_state(pdev->xdev, XenbusStateReconfigured);
That is OKish, thought I am not too happy about us holding the lock.
>
> out:
> mutex_unlock(&pdev->dev_lock);
> @@ -640,10 +632,7 @@ static int xen_pcibk_setup_backend(struct xen_pcibk_device *pdev)
> goto out;
> }
>
> - err = xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
> - if (err)
> - xenbus_dev_fatal(pdev->xdev, err,
> - "Error switching to initialised state!");
> + xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
And this one needs that comment above about the mutex.
>
> out:
> mutex_unlock(&pdev->dev_lock);
> @@ -683,9 +672,7 @@ static int xen_pcibk_xenbus_probe(struct xenbus_device *dev,
> }
>
> /* wait for xend to configure us */
> - err = xenbus_switch_state(dev, XenbusStateInitWait);
> - if (err)
> - goto out;
> + xenbus_switch_state(dev, XenbusStateInitWait);
That means the error that is returned on 'probe' is always zero. I don't
think that is right as we did fail to establish a connection.
>
> /* watch the backend node for backend configuration information */
> err = xenbus_watch_path(dev, dev->nodename, &pdev->be_watch,
> diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
> index ad11258..847bc9c 100644
> --- a/drivers/xen/xen-scsiback.c
> +++ b/drivers/xen/xen-scsiback.c
> @@ -1225,10 +1225,7 @@ static int scsiback_probe(struct xenbus_device *dev,
> if (err)
> xenbus_dev_error(dev, err, "writing feature-sg-grant");
>
> - err = xenbus_switch_state(dev, XenbusStateInitWait);
> - if (err)
> - goto fail;
> -
> + xenbus_switch_state(dev, XenbusStateInitWait);
> return 0;
>
> fail:
^ permalink raw reply related
* Re: [PATCH v2 net] net: Always untag vlan-tagged traffic on input.
From: Nicolas Dichtel @ 2014-09-29 13:58 UTC (permalink / raw)
To: vyasevic, Jiri Pirko
Cc: netdev, Patrick McHardy, Nithin Nayak Sujir, Michael Chan
In-Reply-To: <542964B2.3090303@redhat.com>
Le 29/09/2014 15:54, Vlad Yasevich a écrit :
> On 09/29/2014 09:46 AM, Nicolas Dichtel wrote:
>> Le 29/09/2014 15:05, Jiri Pirko a écrit :
>>> Mon, Sep 29, 2014 at 02:44:57PM CEST, nicolas.dichtel@6wind.com wrote:
>>>> Le 08/08/2014 20:42, Vladislav Yasevich a écrit :
>>>> [snip]
>>>>>
>>>>> The patch attempt to fix this another way. It moves the vlan header
>>>>> stipping code out of the vlan module and always builds it into the
>>>>> kernel network core. This way, even if vlan is not supported on
>>>>> a virtualizatoin host, the virtual machines running on top of such
>>>>> host will still work with VLANs enabled.
>>>> After this patch (0d5501c1c828 ("net: Always untag vlan-tagged traffic on
>>>> input.")), tcpdump will not be able to display the VLAN header on input path.
>>>> Is this really intended?
>>>
>>> I assume you are talking about the case when CONFIG_VLAN_8021Q is "n"
>>> right? Because if it is "m" of "y" I see no change in the code.
>> Right.
>>
>>>
>>> In case of CONFIG_VLAN_8021Q is "n" the only change is that for
>>> non-accelerated path, skb->vlan_tci is set and vlan header is stripped.
>>> This makes the path same to accelerated path.
>>>
>>> tcpdump should be able to cope with skb->vlan_tci
>> My fault, I've tested with a quite old tcpdump version, with a recent one it
>> works perfectly!
>
> Thanks. BTW, did that old tcpdump version work correctly when CONFIG_VLAN_8021Q
> was y or m? My guess is that it would not.
Your guess is right ;-)
Nicolas
^ permalink raw reply
* Re: [PATCH v2 net] net: Always untag vlan-tagged traffic on input.
From: Vlad Yasevich @ 2014-09-29 13:54 UTC (permalink / raw)
To: nicolas.dichtel, Jiri Pirko
Cc: netdev, Patrick McHardy, Nithin Nayak Sujir, Michael Chan
In-Reply-To: <542962CF.6000602@6wind.com>
On 09/29/2014 09:46 AM, Nicolas Dichtel wrote:
> Le 29/09/2014 15:05, Jiri Pirko a écrit :
>> Mon, Sep 29, 2014 at 02:44:57PM CEST, nicolas.dichtel@6wind.com wrote:
>>> Le 08/08/2014 20:42, Vladislav Yasevich a écrit :
>>> [snip]
>>>>
>>>> The patch attempt to fix this another way. It moves the vlan header
>>>> stipping code out of the vlan module and always builds it into the
>>>> kernel network core. This way, even if vlan is not supported on
>>>> a virtualizatoin host, the virtual machines running on top of such
>>>> host will still work with VLANs enabled.
>>> After this patch (0d5501c1c828 ("net: Always untag vlan-tagged traffic on
>>> input.")), tcpdump will not be able to display the VLAN header on input path.
>>> Is this really intended?
>>
>> I assume you are talking about the case when CONFIG_VLAN_8021Q is "n"
>> right? Because if it is "m" of "y" I see no change in the code.
> Right.
>
>>
>> In case of CONFIG_VLAN_8021Q is "n" the only change is that for
>> non-accelerated path, skb->vlan_tci is set and vlan header is stripped.
>> This makes the path same to accelerated path.
>>
>> tcpdump should be able to cope with skb->vlan_tci
> My fault, I've tested with a quite old tcpdump version, with a recent one it
> works perfectly!
Thanks. BTW, did that old tcpdump version work correctly when CONFIG_VLAN_8021Q
was y or m? My guess is that it would not.
-vlad
>
>
> Thank you,
> Nicolas
^ permalink raw reply
* Re: [PATCH v2 net] net: Always untag vlan-tagged traffic on input.
From: Nicolas Dichtel @ 2014-09-29 13:46 UTC (permalink / raw)
To: Jiri Pirko
Cc: Vladislav Yasevich, netdev, Patrick McHardy, Nithin Nayak Sujir,
Michael Chan
In-Reply-To: <20140929130543.GB1839@nanopsycho.orion>
Le 29/09/2014 15:05, Jiri Pirko a écrit :
> Mon, Sep 29, 2014 at 02:44:57PM CEST, nicolas.dichtel@6wind.com wrote:
>> Le 08/08/2014 20:42, Vladislav Yasevich a écrit :
>> [snip]
>>>
>>> The patch attempt to fix this another way. It moves the vlan header
>>> stipping code out of the vlan module and always builds it into the
>>> kernel network core. This way, even if vlan is not supported on
>>> a virtualizatoin host, the virtual machines running on top of such
>>> host will still work with VLANs enabled.
>> After this patch (0d5501c1c828 ("net: Always untag vlan-tagged traffic on
>> input.")), tcpdump will not be able to display the VLAN header on input path.
>> Is this really intended?
>
> I assume you are talking about the case when CONFIG_VLAN_8021Q is "n"
> right? Because if it is "m" of "y" I see no change in the code.
Right.
>
> In case of CONFIG_VLAN_8021Q is "n" the only change is that for
> non-accelerated path, skb->vlan_tci is set and vlan header is stripped.
> This makes the path same to accelerated path.
>
> tcpdump should be able to cope with skb->vlan_tci
My fault, I've tested with a quite old tcpdump version, with a recent one it
works perfectly!
Thank you,
Nicolas
^ permalink raw reply
* Re: [PATCH RFC ipsec-next] xfrm: Add sysctl option to enforce inbound policies for transport mode
From: Herbert Xu @ 2014-09-29 13:46 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Tobias Brunner, davem, netdev, Alexey Kuznetsov
In-Reply-To: <20140919092437.GX6390@secunet.com>
On Fri, Sep 19, 2014 at 11:24:38AM +0200, Steffen Klassert wrote:
> Ccing Herbert Xu.
>
> On Tue, Sep 16, 2014 at 12:49:39PM +0200, Tobias Brunner wrote:
> > Currently inbound policies for transport mode SAs are not enforced.
> > If no policy is found or if the templates don't match this is not
> > considered an error for transport mode SAs.
> >
>
> The strict inbound policy enforcement was implemented by Herbert.
> The commit predates our git history but can be found in the history
> tree:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
>
> It was the following commit:
>
> commit 8fe7ee2ba983fd89b2555dce5930ffd0f7f6c361
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Thu Oct 23 14:57:11 2003 -0700
>
> [IPSEC]: Strengthen policy checks.
>
> Maybe Herbert remembers why this was done only for tunnel mode.
Yes I remember :) Please refer to the following thread:
http://www.spinics.net/lists/linux-net/msg07342.html
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* ASIX 88772
From: Stam, Michel [FINT] @ 2014-09-29 13:45 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 1324 bytes --]
Dear list,
A while back we did an upgrade of the firmware running on our embedded
devices. These devices, amongst others, use an ASIX chip, model 88772A.
What we noticed, was that ethtool settings would be negated when the
interface was set to 'up'. This, effectively voided any control we tried
to exercise over the autonegotiation process (it always returns back to
100 Mbps/Full duplex). After comparing with the kernel we used before
(we used 2.6.22 before, now 3.10.34), I discovered that the difference
was the .link_reset function pointer defined in the driver_info struct
in drivers/net/usb/asix_devices.c:888. By setting it back to its
previous value, ax88772_link_reset, the functionality is restored and
ethtool behaves as expected.
Apparently this change to drivers/net/usb/asix_devices.c happened at
line 888 as part of commit 2e55cc721, which moved the driver into its
own file with that commit. It apparently also addresses some link reset
problems.
Would anyone have any idea what kind of link reset problems these were?
I have attached a git format-patch which works for us, but I would like
to make sure it does not break other devices instead. I've attached it
to this email because the mail client seems to corrupt the patches
occasionally.
Best regards,
Michel Stam
[-- Attachment #2: 0001-Don-t-reset-PHY-on-if_up-for-ASIX-88772.patch --]
[-- Type: application/octet-stream, Size: 1252 bytes --]
From feb298ea0b5733bcbc3171c6c477f55d6fe9cdcf Mon Sep 17 00:00:00 2001
From: Michel Stam <m.stam@fugro.nl>
Date: Tue, 23 Sep 2014 17:20:23 +0200
Subject: [PATCH] Don't reset PHY on if_up for ASIX 88772
I've noticed every time the interface is set to 'up,', the kernel
reports that the link speed is set to 100 Mbps/Full Duplex, even
when ethtool is used to set autonegotiation to 'off', half
duplex, 10 Mbps.
It can be tested by:
ifconfig eth0 down
ethtool -s eth0 autoneg off speed 10 duplex half
ifconfig eth0 up
Then checking 'dmesg' for the link speed.
Signed-off-by: Michel Stam <m.stam@fugro.nl>
---
drivers/net/usb/asix_devices.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 5d19409..2c05f6c 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -890,7 +890,7 @@ static const struct driver_info ax88772_info = {
.unbind = ax88772_unbind,
.status = asix_status,
.link_reset = ax88772_link_reset,
- .reset = ax88772_reset,
+ .reset = ax88772_link_reset,
.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR | FLAG_MULTI_PACKET,
.rx_fixup = asix_rx_fixup_common,
.tx_fixup = asix_tx_fixup,
--
1.7.12.1
^ permalink raw reply related
* Re: BCM4313 & brcmsmac & 3.12: only semi-working?
From: Arend van Spriel @ 2014-09-29 13:44 UTC (permalink / raw)
To: Michael Tokarev; +Cc: Seth Forshee, brcm80211-dev-list, linux-wireless, netdev
In-Reply-To: <54258459.8020803@msgid.tls.msk.ru>
On 09/26/14 17:20, Michael Tokarev wrote:
> I can send it your way, -- guess it will be quite a bit costly,
> but I don't have any use for it anyway (short of throwing it
> away), and since I already spent significantly more money due
> to all this (whole thinkpad plus ssds and several wifi adaptors),
> this additional cost is just a small noize. But since that's
> 2nd card in a row, maybe there's something else in there, the
> prob is not in the card?
Could be. Maybe some BIOS issue. Can you make some hi-res pictures of
the card and email them to me? If it is identical to what I already have
over here there is not much sense in sending it.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH RFC ipsec-next] xfrm: Add sysctl option to enforce inbound policies for transport mode
From: Tobias Brunner @ 2014-09-29 13:39 UTC (permalink / raw)
To: Steffen Klassert; +Cc: davem, netdev, Herbert Xu
In-Reply-To: <20140919092437.GX6390@secunet.com>
>> Currently inbound policies for transport mode SAs are not enforced.
>> If no policy is found or if the templates don't match this is not
>> considered an error for transport mode SAs.
>>
>
> The strict inbound policy enforcement was implemented by Herbert.
> The commit predates our git history but can be found in the history
> tree:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
>
> It was the following commit:
>
> commit 8fe7ee2ba983fd89b2555dce5930ffd0f7f6c361
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Thu Oct 23 14:57:11 2003 -0700
>
> [IPSEC]: Strengthen policy checks.
>
> Maybe Herbert remembers why this was done only for tunnel mode.
Would be great to hear from Herbert about this.
> If I read section 5.2.1 of RFC 2401 correct, the inbound policy
> must be enforced regardless of the mode.
It looks like the wording changed with RFC 4301. The SPD and its
policies are not mentioned explicitly anymore in section 5.2 (like
they were in step 3 in section 5.2.1 of RFC 2401). Instead, packets
must be matched against the "selectors identified by the SAD entry".
It's not entirely clear to me whether these selectors are part of the
SPD or properties of the SAD entries themselves, like the single
selector that can currently be configured on SAs in the kernel.
Also, section 4.4.2 explicitly states that manually keyed SAD entries
do not necessarily need to have a corresponding SPD entry. Which might
make sense for simple host-host (transport mode) SAs, but this wouldn't
be possible anymore when enforcing inbound policies for transport mode.
>> The new sysctl option (net.core.xfrm_enforce_policies_transport_mode)
>> allows enforcing the inbound policies also for transport mode SAs.
>> By default this option remains disabled.
>
> I'd not like to have a sysctl for this. I consider it as a bug
> if an installed policy can not be enforced, and we don't fix bugs
> with sysctls :).
Alright, the patch below simplifies the whole thing by treating
transport mode policies like those in other modes.
----
Subject: [PATCH] xfrm: Enforce inbound transport mode policies like those in other modes
Currently inbound policies for transport mode SAs are not enforced.
If no policy is found or if the templates don't match this is not
considered an error for transport mode SAs.
With these changes the inbound policies are also enforced for transport
mode SAs like they are already for other modes.
Signed-off-by: Tobias Brunner <tobias@strongswan.org>
---
net/xfrm/xfrm_policy.c | 38 ++++++++++----------------------------
1 file changed, 10 insertions(+), 28 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 55bcb8604bc6..74ce57e7b53b 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2348,9 +2348,8 @@ xfrm_state_ok(const struct xfrm_tmpl *tmpl, const struct xfrm_state *x,
}
/*
- * 0 or more than 0 is returned when validation is succeeded (either bypass
- * because of optional transport mode, or next index of the mathced secpath
- * state with the template.
+ * 0 or more than 0 is returned when validation succeeded (next index of the
+ * matched secpath state with the template).
* -1 is returned when no matching template is found.
* Otherwise "-2 - errored_index" is returned.
*/
@@ -2360,19 +2359,13 @@ xfrm_policy_ok(const struct xfrm_tmpl *tmpl, const struct sec_path *sp, int star
{
int idx = start;
- if (tmpl->optional) {
- if (tmpl->mode == XFRM_MODE_TRANSPORT)
- return start;
- } else
+ if (!tmpl->optional)
start = -1;
- for (; idx < sp->len; idx++) {
+ if (idx < sp->len) {
if (xfrm_state_ok(tmpl, sp->xvec[idx], family))
return ++idx;
- if (sp->xvec[idx]->props.mode != XFRM_MODE_TRANSPORT) {
- if (start == -1)
- start = -2-idx;
- break;
- }
+ if (start == -1)
+ start = -2-idx;
}
return start;
}
@@ -2393,18 +2386,6 @@ int __xfrm_decode_session(struct sk_buff *skb, struct flowi *fl,
}
EXPORT_SYMBOL(__xfrm_decode_session);
-static inline int secpath_has_nontransport(const struct sec_path *sp, int k, int *idxp)
-{
- for (; k < sp->len; k++) {
- if (sp->xvec[k]->props.mode != XFRM_MODE_TRANSPORT) {
- *idxp = k;
- return 1;
- }
- }
-
- return 0;
-}
-
int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
unsigned short family)
{
@@ -2469,8 +2450,8 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
}
if (!pol) {
- if (skb->sp && secpath_has_nontransport(skb->sp, 0, &xerr_idx)) {
- xfrm_secpath_reject(xerr_idx, skb, &fl);
+ if (skb->sp && skb->sp->len) {
+ xfrm_secpath_reject(0, skb, &fl);
XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOPOLS);
return 0;
}
@@ -2545,7 +2526,8 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
}
}
- if (secpath_has_nontransport(sp, k, &xerr_idx)) {
+ if (k < sp->len) {
+ xerr_idx = k;
XFRM_INC_STATS(net, LINUX_MIB_XFRMINTMPLMISMATCH);
goto reject;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] xen/xenbus: Use 'void' instead of 'int' for the return of xenbus_switch_state()
From: Bjorn Helgaas @ 2014-09-29 13:35 UTC (permalink / raw)
To: Chen Gang
Cc: David Vrabel, Konrad Rzeszutek Wilk, Ian Campbell, wei.liu2,
Boris Ostrovsky, jgross, Wei Yongjun, mukesh.rathor,
xen-devel@lists.xenproject.org, netdev@vger.kernel.org,
linux-pci@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <5425961A.5000604@gmail.com>
On Fri, Sep 26, 2014 at 10:36 AM, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
> When xenbus_switch_state() fails, it will call xenbus_switch_fatal()
> internally, so need not return any status value, then use 'void' instead
> of 'int' for xenbus_switch_state() and __xenbus_switch_state().
>
> Also need be sure that all callers which check the return value must let
> 'err' be 0.
>
> And also need change the related comments for xenbus_switch_state().
>
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> ---
> drivers/block/xen-blkback/xenbus.c | 9 ++-------
> drivers/net/xen-netback/xenbus.c | 5 +----
> drivers/pci/xen-pcifront.c | 15 ++++++---------
> drivers/xen/xen-pciback/xenbus.c | 21 ++++-----------------
> drivers/xen/xen-scsiback.c | 5 +----
> drivers/xen/xenbus/xenbus_client.c | 16 ++++++++--------
> include/xen/xenbus.h | 3 ++-
> 7 files changed, 24 insertions(+), 50 deletions(-)
I don't know enough to review this, but as far as I'm concerned, the
Xen folks own drivers/pci/xen-pcifront.c, so whatever they want to do
is fine with me.
Bjorn
^ permalink raw reply
* Re: [PATCH v2 3/4] ptp: Add PTP_PF_PPS enumeration to ptp_pin_function.
From: Richard Cochran @ 2014-09-29 13:25 UTC (permalink / raw)
To: luwei.zhou@freescale.com
Cc: davem@davemloft.net, netdev@vger.kernel.org, shawn.guo@linaro.org,
bhutchings@solarflare.com, Fabio.Estevam@freescale.com,
fugang.duan@freescale.com, Frank.Li@freescale.com,
stephen@networkplumber.org
In-Reply-To: <ee73b9239ea9417e976fdf2c51b2fd60@BY2PR03MB441.namprd03.prod.outlook.com>
On Mon, Sep 29, 2014 at 09:38:44AM +0000, luwei.zhou@freescale.com wrote:
> My understanding is n_pins should be 1 because there is only one PPS output
The field 'n_pins' is the number of *physical* *programmable* pins.
If the PPS signal on your SoC can appear on any of 4 different pins,
then n_pins should be 4.
If the PPS signal on your SoC can only appear on one pin,
then n_pins should be 0 (zero).
Please see:
Documentation/ptp/testptp.c
include/linux/ptp_clock_kernel.h
include/uapi/linux/ptp_clock.h
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH v2 1/1] net: fec: implement rx_copybreak to improve rx performance
From: Zhi Li @ 2014-09-29 13:09 UTC (permalink / raw)
To: fugang.duan@freescale.com
Cc: Eric Dumazet, Frank.Li@freescale.com, davem@davemloft.net,
romieu@fr.zoreil.com, netdev@vger.kernel.org,
shawn.guo@linaro.org, bhutchings@solarflare.com
In-Reply-To: <323020b0c5a44235a1707dd0619deba3@BLUPR03MB373.namprd03.prod.outlook.com>
On Mon, Sep 29, 2014 at 7:43 AM, fugang.duan@freescale.com
<fugang.duan@freescale.com> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com> Sent: Monday, September 29, 2014 8:12 PM
>>To: Duan Fugang-B38611
>>Cc: Li Frank-B20596; davem@davemloft.net; romieu@fr.zoreil.com;
>>netdev@vger.kernel.org; shawn.guo@linaro.org; bhutchings@solarflare.com
>>Subject: Re: [PATCH v2 1/1] net: fec: implement rx_copybreak to improve rx
>>performance
>>
>>On Mon, 2014-09-29 at 15:47 +0800, Fugang Duan wrote:
>>> - Copy short frames and keep the buffers mapped, re-allocate skb instead
>>of
>>> memory copy for long frames.
>>> - Add support for setting/getting rx_copybreak using generic ethtool
>>> tunable
>>>
>>> Changes V2:
>>> * Implements rx_copybreak
>>
>>>
>>> +#define COPYBREAK_DEFAULT 256
>>> +static unsigned int copybreak __read_mostly = COPYBREAK_DEFAULT;
>>> +module_param(copybreak, uint, 0644); MODULE_PARM_DESC(copybreak,
>>> + "Maximum packet size that is copied to a new buffer on
>>receive");
>>> +
>>
>>It is not the right way to handle this.
>>
>>Old drivers might still have a legacy module parameter.
>>
>>New implementations should provide the new ethtool support, and only this.
>>No new module parameter is accepted.
>>
>>For details, check following commits :
>>
>>d4ad30b182305ecf97f145a5d4d1fd9e728c6d01 enic: Add tunable_ops support for
>>rx_copybreak
>>
>>f0db9b073415848709dd59a6394969882f517da9 ethtool: Add generic options for
>>tunables
>>
>>
>>Or check the current tree :
>>
>>git grep -n rx_copybreak drivers/net/ethernet/cisco/enic
>>
> The patch implement ethtool interface, pls read the patch completely.
I think Eric's means is that REMOVE module parameter and just keep ethtool API.
best regards
Frank Li
>
> Thanks,
> Andy
^ permalink raw reply
* Re: VRFs and the scalability of namespaces
From: David Ahern @ 2014-09-29 13:06 UTC (permalink / raw)
To: Hannes Frederic Sowa, Eric W. Biederman; +Cc: nicolas.dichtel, netdev
In-Reply-To: <1411824598.2136890.172383085.705271DD@webmail.messagingengine.com>
Hi Hannes:
On 9/27/14, 7:29 AM, Hannes Frederic Sowa wrote:
> Did you already did an investigation how maybe the rule and table
> features could be exploited to suite your needs? Some time back I
I did look into the existing multiple table option but not to the extent
of creating a POC. It has been on my to-do list for 4+ months now I just
have not had time to get to it. Based on a number of Google searches to
review the history of VRFs and the kernel, I did see the use of multiple
routing tables has been suggested as well and its problems have been
delineated. e.g.,
http://www.spinics.net/lists/linux-net/msg17502.html
> suggested something like "ip route table foo exec ....", keep an default
> routing lookup indicator in task_struct which gets implicitly propagated
> to rtnetlink routing table requests/modification for the requested
> table. Tables already can be specified via rtnetlink, so no change would
> be needed here.
>
> For sockets something like SO_BINDTOTABLE might work, maybe even we can
> by default use the task_struct information to also bind the sockets to
> the per-process table. We certainly need to preserve the routing
> information on the socket as we need those in icmp error handling (e.g.
> where to apply ipv4/ipv6 redirects too). Directing incoming packets to
> specific table also works via ip-rule-iif match.
>
> Advantage with the ip route table foo exec... method would be, that
> conversion of some unmodified routing management daemons might be
> easier, others can either use rtnetlink extended attributes which are
> already available, and we only need to have per-process context routing
> table control, which seems not too hard to implement in ip-rule
> subsystem, but I haven't checked.
>
> The problem I see with rules is that some of those tables already work
> hand in hand, they already have a implicit semantics, e.g. local, main,
> default and unspec (this is even worse for IPv6, where addrconf already
> uses hardcoded tables). Working around this might be very tricky and
> even more problematic to do from user space.
>
> I think I am not yet sure what features you want from VRFs, some things
> seem to match the rule/table features but others I think are pretty hard
> to implement.
The features of note:
- resource efficiency -- not having to create a proces/thread/socket per
VRF to have a "presence" in all VRFs. e.g., a VRF any context that
allows 1 socket to work across VRFs (L3 raw socket, TCP listen socket,
unconnected UDP socket). Daemons run a 'vrf any' context; connected
clients run a specific vrf context. For non-connected sockets VRF
context can be passed via cmsg.
- same IP address on different interfaces in different vrfs. i.e., VRF
specific routing and neighbor tables
- cross VRF routing. ability to receive message on 1 vrf and send it on
another. Can be handled by the process itself (e.g., L3 vpns).
Thanks,
David
^ permalink raw reply
* Re: [PATCH v2 net] net: Always untag vlan-tagged traffic on input.
From: Jiri Pirko @ 2014-09-29 13:05 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: Vladislav Yasevich, netdev, Patrick McHardy, Nithin Nayak Sujir,
Michael Chan
In-Reply-To: <54295449.7030203@6wind.com>
Mon, Sep 29, 2014 at 02:44:57PM CEST, nicolas.dichtel@6wind.com wrote:
>Le 08/08/2014 20:42, Vladislav Yasevich a écrit :
>[snip]
>>
>>The patch attempt to fix this another way. It moves the vlan header
>>stipping code out of the vlan module and always builds it into the
>>kernel network core. This way, even if vlan is not supported on
>>a virtualizatoin host, the virtual machines running on top of such
>>host will still work with VLANs enabled.
>After this patch (0d5501c1c828 ("net: Always untag vlan-tagged traffic on
>input.")), tcpdump will not be able to display the VLAN header on input path.
>Is this really intended?
I assume you are talking about the case when CONFIG_VLAN_8021Q is "n"
right? Because if it is "m" of "y" I see no change in the code.
In case of CONFIG_VLAN_8021Q is "n" the only change is that for
non-accelerated path, skb->vlan_tci is set and vlan header is stripped.
This makes the path same to accelerated path.
tcpdump should be able to cope with skb->vlan_tci
>
>
>Regards,
>Nicolas
^ permalink raw reply
* RE: [PATCH v2 1/1] net: fec: implement rx_copybreak to improve rx performance
From: fugang.duan @ 2014-09-29 12:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: Frank.Li@freescale.com, davem@davemloft.net, romieu@fr.zoreil.com,
netdev@vger.kernel.org, shawn.guo@linaro.org,
bhutchings@solarflare.com
In-Reply-To: <1411992739.30721.23.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com> Sent: Monday, September 29, 2014 8:12 PM
>To: Duan Fugang-B38611
>Cc: Li Frank-B20596; davem@davemloft.net; romieu@fr.zoreil.com;
>netdev@vger.kernel.org; shawn.guo@linaro.org; bhutchings@solarflare.com
>Subject: Re: [PATCH v2 1/1] net: fec: implement rx_copybreak to improve rx
>performance
>
>On Mon, 2014-09-29 at 15:47 +0800, Fugang Duan wrote:
>> - Copy short frames and keep the buffers mapped, re-allocate skb instead
>of
>> memory copy for long frames.
>> - Add support for setting/getting rx_copybreak using generic ethtool
>> tunable
>>
>> Changes V2:
>> * Implements rx_copybreak
>
>>
>> +#define COPYBREAK_DEFAULT 256
>> +static unsigned int copybreak __read_mostly = COPYBREAK_DEFAULT;
>> +module_param(copybreak, uint, 0644); MODULE_PARM_DESC(copybreak,
>> + "Maximum packet size that is copied to a new buffer on
>receive");
>> +
>
>It is not the right way to handle this.
>
>Old drivers might still have a legacy module parameter.
>
>New implementations should provide the new ethtool support, and only this.
>No new module parameter is accepted.
>
>For details, check following commits :
>
>d4ad30b182305ecf97f145a5d4d1fd9e728c6d01 enic: Add tunable_ops support for
>rx_copybreak
>
>f0db9b073415848709dd59a6394969882f517da9 ethtool: Add generic options for
>tunables
>
>
>Or check the current tree :
>
>git grep -n rx_copybreak drivers/net/ethernet/cisco/enic
>
The patch implement ethtool interface, pls read the patch completely.
Thanks,
Andy
^ permalink raw reply
* Re: [PATCH v2 net] net: Always untag vlan-tagged traffic on input.
From: Nicolas Dichtel @ 2014-09-29 12:44 UTC (permalink / raw)
To: Vladislav Yasevich, netdev
Cc: Patrick McHardy, Nithin Nayak Sujir, Michael Chan, Jiri Pirko
In-Reply-To: <1407523333-31455-1-git-send-email-vyasevic@redhat.com>
Le 08/08/2014 20:42, Vladislav Yasevich a écrit :
[snip]
>
> The patch attempt to fix this another way. It moves the vlan header
> stipping code out of the vlan module and always builds it into the
> kernel network core. This way, even if vlan is not supported on
> a virtualizatoin host, the virtual machines running on top of such
> host will still work with VLANs enabled.
After this patch (0d5501c1c828 ("net: Always untag vlan-tagged traffic on
input.")), tcpdump will not be able to display the VLAN header on input path.
Is this really intended?
Regards,
Nicolas
^ permalink raw reply
* [PATCH 30/34] net/netfilter/x_tables.c: use __seq_open_private()
From: Pablo Neira Ayuso @ 2014-09-29 12:39 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1411994363-8451-1-git-send-email-pablo@netfilter.org>
From: Rob Jones <rob.jones@codethink.co.uk>
Reduce boilerplate code by using __seq_open_private() instead of seq_open()
in xt_match_open() and xt_target_open().
Signed-off-by: Rob Jones <rob.jones@codethink.co.uk>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/x_tables.c | 30 ++++--------------------------
1 file changed, 4 insertions(+), 26 deletions(-)
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 272ae4d..133eb47 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1101,22 +1101,11 @@ static const struct seq_operations xt_match_seq_ops = {
static int xt_match_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
struct nf_mttg_trav *trav;
- int ret;
-
- trav = kmalloc(sizeof(*trav), GFP_KERNEL);
- if (trav == NULL)
+ trav = __seq_open_private(file, &xt_match_seq_ops, sizeof(*trav));
+ if (!trav)
return -ENOMEM;
- ret = seq_open(file, &xt_match_seq_ops);
- if (ret < 0) {
- kfree(trav);
- return ret;
- }
-
- seq = file->private_data;
- seq->private = trav;
trav->nfproto = (unsigned long)PDE_DATA(inode);
return 0;
}
@@ -1165,22 +1154,11 @@ static const struct seq_operations xt_target_seq_ops = {
static int xt_target_open(struct inode *inode, struct file *file)
{
- struct seq_file *seq;
struct nf_mttg_trav *trav;
- int ret;
-
- trav = kmalloc(sizeof(*trav), GFP_KERNEL);
- if (trav == NULL)
+ trav = __seq_open_private(file, &xt_target_seq_ops, sizeof(*trav));
+ if (!trav)
return -ENOMEM;
- ret = seq_open(file, &xt_target_seq_ops);
- if (ret < 0) {
- kfree(trav);
- return ret;
- }
-
- seq = file->private_data;
- seq->private = trav;
trav->nfproto = (unsigned long)PDE_DATA(inode);
return 0;
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 27/34] ipvs: Allow heterogeneous pools now that we support them
From: Pablo Neira Ayuso @ 2014-09-29 12:39 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1411994363-8451-1-git-send-email-pablo@netfilter.org>
From: Alex Gartrell <agartrell@fb.com>
Remove the temporary consistency check and add a case statement to only
allow ipip mixed dests.
Signed-off-by: Alex Gartrell <agartrell@fb.com>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_ctl.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 462760e..ac7ba68 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -854,10 +854,6 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest,
EnterFunction(2);
- /* Temporary for consistency */
- if (udest->af != svc->af)
- return -EINVAL;
-
#ifdef CONFIG_IP_VS_IPV6
if (udest->af == AF_INET6) {
atype = ipv6_addr_type(&udest->addr.in6);
@@ -3403,6 +3399,26 @@ static int ip_vs_genl_set_cmd(struct sk_buff *skb, struct genl_info *info)
*/
if (udest.af == 0)
udest.af = svc->af;
+
+ if (udest.af != svc->af) {
+ /* The synchronization protocol is incompatible
+ * with mixed family services
+ */
+ if (net_ipvs(net)->sync_state) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* Which connection types do we support? */
+ switch (udest.conn_flags) {
+ case IP_VS_CONN_F_TUNNEL:
+ /* We are able to forward this */
+ break;
+ default:
+ ret = -EINVAL;
+ goto out;
+ }
+ }
}
switch (cmd) {
--
1.7.10.4
^ permalink raw reply related
* [PATCH 26/34] ipvs: use the new dest addr family field
From: Pablo Neira Ayuso @ 2014-09-29 12:39 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1411994363-8451-1-git-send-email-pablo@netfilter.org>
From: Julian Anastasov <ja@ssi.bg>
Use the new address family field cp->daf when printing
cp->daddr in logs or connection listing.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Alex Gartrell <agartrell@fb.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_conn.c | 49 +++++++++++++++++++++++++--------
net/netfilter/ipvs/ip_vs_core.c | 6 ++--
net/netfilter/ipvs/ip_vs_proto_sctp.c | 2 +-
net/netfilter/ipvs/ip_vs_proto_tcp.c | 2 +-
4 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 13e9cee..b0f7b62 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -27,6 +27,7 @@
#include <linux/interrupt.h>
#include <linux/in.h>
+#include <linux/inet.h>
#include <linux/net.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -77,6 +78,13 @@ static unsigned int ip_vs_conn_rnd __read_mostly;
#define CT_LOCKARRAY_SIZE (1<<CT_LOCKARRAY_BITS)
#define CT_LOCKARRAY_MASK (CT_LOCKARRAY_SIZE-1)
+/* We need an addrstrlen that works with or without v6 */
+#ifdef CONFIG_IP_VS_IPV6
+#define IP_VS_ADDRSTRLEN INET6_ADDRSTRLEN
+#else
+#define IP_VS_ADDRSTRLEN (8+1)
+#endif
+
struct ip_vs_aligned_lock
{
spinlock_t l;
@@ -588,7 +596,7 @@ ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest)
ip_vs_proto_name(cp->protocol),
IP_VS_DBG_ADDR(cp->af, &cp->caddr), ntohs(cp->cport),
IP_VS_DBG_ADDR(cp->af, &cp->vaddr), ntohs(cp->vport),
- IP_VS_DBG_ADDR(cp->af, &cp->daddr), ntohs(cp->dport),
+ IP_VS_DBG_ADDR(cp->daf, &cp->daddr), ntohs(cp->dport),
ip_vs_fwd_tag(cp), cp->state,
cp->flags, atomic_read(&cp->refcnt),
atomic_read(&dest->refcnt));
@@ -685,7 +693,7 @@ static inline void ip_vs_unbind_dest(struct ip_vs_conn *cp)
ip_vs_proto_name(cp->protocol),
IP_VS_DBG_ADDR(cp->af, &cp->caddr), ntohs(cp->cport),
IP_VS_DBG_ADDR(cp->af, &cp->vaddr), ntohs(cp->vport),
- IP_VS_DBG_ADDR(cp->af, &cp->daddr), ntohs(cp->dport),
+ IP_VS_DBG_ADDR(cp->daf, &cp->daddr), ntohs(cp->dport),
ip_vs_fwd_tag(cp), cp->state,
cp->flags, atomic_read(&cp->refcnt),
atomic_read(&dest->refcnt));
@@ -754,7 +762,7 @@ int ip_vs_check_template(struct ip_vs_conn *ct)
ntohs(ct->cport),
IP_VS_DBG_ADDR(ct->af, &ct->vaddr),
ntohs(ct->vport),
- IP_VS_DBG_ADDR(ct->af, &ct->daddr),
+ IP_VS_DBG_ADDR(ct->daf, &ct->daddr),
ntohs(ct->dport));
/*
@@ -1051,6 +1059,7 @@ static int ip_vs_conn_seq_show(struct seq_file *seq, void *v)
struct net *net = seq_file_net(seq);
char pe_data[IP_VS_PENAME_MAXLEN + IP_VS_PEDATA_MAXLEN + 3];
size_t len = 0;
+ char dbuf[IP_VS_ADDRSTRLEN];
if (!ip_vs_conn_net_eq(cp, net))
return 0;
@@ -1065,24 +1074,32 @@ static int ip_vs_conn_seq_show(struct seq_file *seq, void *v)
pe_data[len] = '\0';
#ifdef CONFIG_IP_VS_IPV6
+ if (cp->daf == AF_INET6)
+ snprintf(dbuf, sizeof(dbuf), "%pI6", &cp->daddr.in6);
+ else
+#endif
+ snprintf(dbuf, sizeof(dbuf), "%08X",
+ ntohl(cp->daddr.ip));
+
+#ifdef CONFIG_IP_VS_IPV6
if (cp->af == AF_INET6)
seq_printf(seq, "%-3s %pI6 %04X %pI6 %04X "
- "%pI6 %04X %-11s %7lu%s\n",
+ "%s %04X %-11s %7lu%s\n",
ip_vs_proto_name(cp->protocol),
&cp->caddr.in6, ntohs(cp->cport),
&cp->vaddr.in6, ntohs(cp->vport),
- &cp->daddr.in6, ntohs(cp->dport),
+ dbuf, ntohs(cp->dport),
ip_vs_state_name(cp->protocol, cp->state),
(cp->timer.expires-jiffies)/HZ, pe_data);
else
#endif
seq_printf(seq,
"%-3s %08X %04X %08X %04X"
- " %08X %04X %-11s %7lu%s\n",
+ " %s %04X %-11s %7lu%s\n",
ip_vs_proto_name(cp->protocol),
ntohl(cp->caddr.ip), ntohs(cp->cport),
ntohl(cp->vaddr.ip), ntohs(cp->vport),
- ntohl(cp->daddr.ip), ntohs(cp->dport),
+ dbuf, ntohs(cp->dport),
ip_vs_state_name(cp->protocol, cp->state),
(cp->timer.expires-jiffies)/HZ, pe_data);
}
@@ -1120,6 +1137,7 @@ static const char *ip_vs_origin_name(unsigned int flags)
static int ip_vs_conn_sync_seq_show(struct seq_file *seq, void *v)
{
+ char dbuf[IP_VS_ADDRSTRLEN];
if (v == SEQ_START_TOKEN)
seq_puts(seq,
@@ -1132,12 +1150,21 @@ static int ip_vs_conn_sync_seq_show(struct seq_file *seq, void *v)
return 0;
#ifdef CONFIG_IP_VS_IPV6
+ if (cp->daf == AF_INET6)
+ snprintf(dbuf, sizeof(dbuf), "%pI6", &cp->daddr.in6);
+ else
+#endif
+ snprintf(dbuf, sizeof(dbuf), "%08X",
+ ntohl(cp->daddr.ip));
+
+#ifdef CONFIG_IP_VS_IPV6
if (cp->af == AF_INET6)
- seq_printf(seq, "%-3s %pI6 %04X %pI6 %04X %pI6 %04X %-11s %-6s %7lu\n",
+ seq_printf(seq, "%-3s %pI6 %04X %pI6 %04X "
+ "%s %04X %-11s %-6s %7lu\n",
ip_vs_proto_name(cp->protocol),
&cp->caddr.in6, ntohs(cp->cport),
&cp->vaddr.in6, ntohs(cp->vport),
- &cp->daddr.in6, ntohs(cp->dport),
+ dbuf, ntohs(cp->dport),
ip_vs_state_name(cp->protocol, cp->state),
ip_vs_origin_name(cp->flags),
(cp->timer.expires-jiffies)/HZ);
@@ -1145,11 +1172,11 @@ static int ip_vs_conn_sync_seq_show(struct seq_file *seq, void *v)
#endif
seq_printf(seq,
"%-3s %08X %04X %08X %04X "
- "%08X %04X %-11s %-6s %7lu\n",
+ "%s %04X %-11s %-6s %7lu\n",
ip_vs_proto_name(cp->protocol),
ntohl(cp->caddr.ip), ntohs(cp->cport),
ntohl(cp->vaddr.ip), ntohs(cp->vport),
- ntohl(cp->daddr.ip), ntohs(cp->dport),
+ dbuf, ntohs(cp->dport),
ip_vs_state_name(cp->protocol, cp->state),
ip_vs_origin_name(cp->flags),
(cp->timer.expires-jiffies)/HZ);
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 1f6ecb7..990decb 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -492,9 +492,9 @@ ip_vs_schedule(struct ip_vs_service *svc, struct sk_buff *skb,
IP_VS_DBG_BUF(6, "Schedule fwd:%c c:%s:%u v:%s:%u "
"d:%s:%u conn->flags:%X conn->refcnt:%d\n",
ip_vs_fwd_tag(cp),
- IP_VS_DBG_ADDR(svc->af, &cp->caddr), ntohs(cp->cport),
- IP_VS_DBG_ADDR(svc->af, &cp->vaddr), ntohs(cp->vport),
- IP_VS_DBG_ADDR(svc->af, &cp->daddr), ntohs(cp->dport),
+ IP_VS_DBG_ADDR(cp->af, &cp->caddr), ntohs(cp->cport),
+ IP_VS_DBG_ADDR(cp->af, &cp->vaddr), ntohs(cp->vport),
+ IP_VS_DBG_ADDR(cp->daf, &cp->daddr), ntohs(cp->dport),
cp->flags, atomic_read(&cp->refcnt));
ip_vs_conn_stats(cp, svc);
diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index 2f7ea75..5b84c0b 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -432,7 +432,7 @@ set_sctp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
pd->pp->name,
((direction == IP_VS_DIR_OUTPUT) ?
"output " : "input "),
- IP_VS_DBG_ADDR(cp->af, &cp->daddr),
+ IP_VS_DBG_ADDR(cp->daf, &cp->daddr),
ntohs(cp->dport),
IP_VS_DBG_ADDR(cp->af, &cp->caddr),
ntohs(cp->cport),
diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c
index e3a6972..8e92beb 100644
--- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
@@ -510,7 +510,7 @@ set_tcp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
th->fin ? 'F' : '.',
th->ack ? 'A' : '.',
th->rst ? 'R' : '.',
- IP_VS_DBG_ADDR(cp->af, &cp->daddr),
+ IP_VS_DBG_ADDR(cp->daf, &cp->daddr),
ntohs(cp->dport),
IP_VS_DBG_ADDR(cp->af, &cp->caddr),
ntohs(cp->cport),
--
1.7.10.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox