* Re: [patch net-next 0/5] fix bonding slave info API (sysfs and netlink)
From: David Miller @ 2014-01-23 5:58 UTC (permalink / raw)
To: jiri
Cc: netdev, fubar, vfalico, andy, sfeldma, stephen, vyasevic,
nicolas.dichtel, john.r.fastabend
In-Reply-To: <1390377957-31466-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 22 Jan 2014 09:05:52 +0100
> The main part of this patchset is the introduction of a generic way how
> to get/set info of slaves unsing rtnl_link_ops.
Series applied.
^ permalink raw reply
* Re: [PATCH 2/2] net/neighbour: queue work on power efficient wq
From: David Miller @ 2014-01-23 5:58 UTC (permalink / raw)
To: viresh.kumar; +Cc: linaro-kernel, patches, netdev, linux-kernel
In-Reply-To: <faa5b965e81fcb4c3261561e58a8ab1194ecf562.1390373223.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Wed, 22 Jan 2014 12:23:33 +0530
> Workqueue used in neighbour layer have no real dependency of scheduling these on
> the cpu which scheduled them.
>
> On a idle system, it is observed that an idle cpu wakes up many times just to
> service this work. It would be better if we can schedule it on a cpu which the
> scheduler believes to be the most appropriate one.
>
> This patch replaces normal workqueues with power efficient versions. This
> doesn't change existing behavior of code unless CONFIG_WQ_POWER_EFFICIENT is
> enabled.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH 1/2] net/ipv4: queue work on power efficient wq
From: David Miller @ 2014-01-23 5:58 UTC (permalink / raw)
To: viresh.kumar; +Cc: linaro-kernel, patches, netdev, linux-kernel
In-Reply-To: <c24546776d8ec32afd1c084dd1d2a72fc96c6519.1390373223.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Wed, 22 Jan 2014 12:23:32 +0530
> Workqueue used in ipv4 layer have no real dependency of scheduling these on the
> cpu which scheduled them.
>
> On a idle system, it is observed that an idle cpu wakes up many times just to
> service this work. It would be better if we can schedule it on a cpu which the
> scheduler believes to be the most appropriate one.
>
> This patch replaces normal workqueues with power efficient versions. This
> doesn't change existing behavior of code unless CONFIG_WQ_POWER_EFFICIENT is
> enabled.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: update comments of "struct msghdr" with the more accurate RFC3542 ones
From: David Miller @ 2014-01-23 5:58 UTC (permalink / raw)
To: fx.lebail; +Cc: netdev
In-Reply-To: <1390365586-4149-1-git-send-email-fx.lebail@yahoo.com>
From: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
Date: Wed, 22 Jan 2014 05:39:46 +0100
> Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
Fair enough, applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v3] ipv6: enable anycast addresses as source addresses for datagrams
From: David Miller @ 2014-01-23 5:57 UTC (permalink / raw)
To: fx.lebail; +Cc: netdev, hannes, dlstevens
In-Reply-To: <1390361450-3198-1-git-send-email-fx.lebail@yahoo.com>
From: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
Date: Wed, 22 Jan 2014 04:30:50 +0100
> This change allows to consider an anycast address valid as source address
> when given via an IPV6_PKTINFO or IPV6_2292PKTINFO ancillary data item.
> So, when sending a datagram with ancillary data, the unicast and anycast
> addresses are handled in the same way.
>
> - Adds ipv6_chk_acast_addr_src() to check if an anycast address is link-local
> on given interface or is global.
> - Uses it in ip6_datagram_send_ctl().
>
> Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
Applied.
^ permalink raw reply
* Re: [PATCH 2/4 ethtool] ethtool: Support for configurable RSS hash key.
From: Ben Hutchings @ 2014-01-23 5:42 UTC (permalink / raw)
To: Venkata Duvvuru; +Cc: netdev@vger.kernel.org
In-Reply-To: <BF3270C86E8B1349A26C34E4EC1C44CB2C84CF8C@CMEXMB1.ad.emulex.com>
[-- Attachment #1: Type: text/plain, Size: 2538 bytes --]
On Wed, 2014-01-22 at 12:06 +0000, Venkata Duvvuru wrote:
> > -----Original Message-----
> > From: Ben Hutchings [mailto:ben@decadent.org.uk]
> > Sent: Monday, January 20, 2014 7:13 PM
> > To: Venkata Duvvuru
> > Cc: netdev@vger.kernel.org
> > Subject: Re: [PATCH 2/4 ethtool] ethtool: Support for configurable RSS hash
> > key.
> >
> > On Mon, 2014-01-20 at 13:28 +0000, Venkata Duvvuru wrote:
> > > Ben, Please ignore my previous reply. My reply options were screwed up in
> > that.
> > >
> > > > -----Original Message-----
> > > > From: Ben Hutchings [mailto:ben@decadent.org.uk]
> > > > Sent: Monday, January 20, 2014 12:06 AM
> > > > To: Venkata Duvvuru
> > > > Cc: netdev@vger.kernel.org
> > > > Subject: Re: [PATCH 2/4 ethtool] ethtool: Support for configurable
> > > > RSS hash key.
> > > >
> > > > On Fri, 2014-01-17 at 13:02 +0000, Venkata Duvvuru wrote:
> > > > > This ethtool patch will primarily implement the parser for the
> > > > > options
> > > > provided by the user for set and get hashkey before invoking the ioctl.
> > > > > This patch also has Ethtool man page changes which describes the
> > > > > Usage of
> > > > set and get hashkey options.
> > > >
> > > > I'd prefer to have this combined with the -x/-X options (and add new
> > > > long options to reflect that they cover the key as well).
> > >
> > > if we add hashkey options to the existing -x/-X (--show-rxfh-indir/ --set-
> > rxfh-indir), I think it won't be appropriate going by the command name.
> > > We could change the command name to something like --show-rssconfig /-
> > -rss-config but I'm afraid would that be backward compatible?
> > [...]
> >
> > That's why I said 'add new long options'. The ethtool argument parser allows
> > arbitrarily many aliases for each sub-command.
>
> Just to make sure that we are in sync
> { "-x|--show-rxfh-indir|--show-hashkey", 1, do_getrssconfig,
> "Show RSS configuration" },
>
> { "-X|--set-rxfh-indir|--hashkey", 1, do_setrssconfig,
> "Set RSS configuration",
> " equal N | weight W0 W1 ...\n"
> " hkey %x:%x:%x:%x:%x:....:%x\n" },
>
> And equal/weight will be mutually exclusive with hkey.
> Does it makes sense?
No, you should be able to set both the key and the indirection table at
once. And the new aliases should then be something like --set-rxfh and
--show-rxfh.
Ben.
--
Ben Hutchings
compatible: Gracefully accepts erroneous data from any source
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 3/4] ethtool: Support for configurable RSS hash key.
From: Ben Hutchings @ 2014-01-23 5:39 UTC (permalink / raw)
To: Venkata Duvvuru; +Cc: netdev@vger.kernel.org
In-Reply-To: <BF3270C86E8B1349A26C34E4EC1C44CB2C84CFAE@CMEXMB1.ad.emulex.com>
[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]
On Wed, 2014-01-22 at 12:12 +0000, Venkata Duvvuru wrote:
[...]
> > No, what I mean is:
> >
> > 1. An RX flow steering filter can specify use of RSS, in which case the value
> > looked up in the indirection is added to the queue number specified in the
> > filter. This is not yet controllable through RX NFC though there is room for
> > extension there.
> >
> > 2. Multi-function controllers need multiple RSS contexts (key + indirection
> > table) to support independent use of RSS on each function.
> > But it may also be possible to allocate multiple contexts to a single function.
> > This could be useful in conjunction with 1. But there would need to be a way
> > to allocate and configure extra contexts first.
> The proposed changes will be incremental so I think this can be done
> in a separate patch. Thoughts?
The ethtool ABI (to userland) has to remain backward-compatible, and it
is preferable if we don't add lots of different structures for this.
So please define the new command structure to include both the key and
indirection table, and some reserved space (documented as 'userland must
set to 0') for future extensions.
Ben.
--
Ben Hutchings
compatible: Gracefully accepts erroneous data from any source
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v3] tuntap: Fix for a race in accessing numqueues
From: David Miller @ 2014-01-23 5:33 UTC (permalink / raw)
To: dominic.curran; +Cc: netdev, jasowang, maxk
In-Reply-To: <1390359803-27989-1-git-send-email-dominic.curran@citrix.com>
From: Dominic Curran <dominic.curran@citrix.com>
Date: Wed, 22 Jan 2014 03:03:23 +0000
> A patch for fixing a race between queue selection and changing queues
> was introduced in commit 92bb73ea2("tuntap: fix a possible race between
> queue selection and changing queues").
>
> The fix was to prevent the driver from re-reading the tun->numqueues
> more than once within tun_select_queue() using ACCESS_ONCE().
>
> We have been experiancing 'Divide-by-zero' errors in tun_net_xmit()
> since we moved from 3.6 to 3.10, and believe that they come from a
> simular source where the value of tun->numqueues changes to zero
> between the first and a subsequent read of tun->numqueues.
>
> The fix is a simular use of ACCESS_ONCE(), as well as a multiply
> instead of a divide in the if statement.
>
> Signed-off-by: Dominic Curran <dominic.curran@citrix.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next] bridge: Remove unnecessary vlan_put_tag in br_handle_vlan
From: David Miller @ 2014-01-23 5:30 UTC (permalink / raw)
To: makita.toshiaki; +Cc: stephen, vyasevic, netdev
In-Reply-To: <1390350577-4122-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Wed, 22 Jan 2014 09:29:37 +0900
> br_handle_vlan() pushes HW accelerated vlan tag into skbuff when outgoing
> port is the bridge device.
> This is unnecessary because __netif_receive_skb_core() can handle skbs
> with HW accelerated vlan tag. In current implementation,
> __netif_receive_skb_core() needs to extract the vlan tag embedded in skb
> data. This could cause low network performance especially when receiving
> frames at a high frame rate on the bridge device.
>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] tcp: metrics: Fix rcu-race when deleting multiple entries
From: David Miller @ 2014-01-23 5:26 UTC (permalink / raw)
To: christoph.paasch; +Cc: netdev
In-Reply-To: <1390307426-10840-1-git-send-email-christoph.paasch@uclouvain.be>
From: Christoph Paasch <christoph.paasch@uclouvain.be>
Date: Tue, 21 Jan 2014 13:30:26 +0100
> In bbf852b96ebdc6d1 I introduced the tmlist, which allows to delete
> multiple entries from the cache that match a specified destination if no
> source-IP is specified.
>
> However, as the cache is an RCU-list, we should not create this tmlist, as
> it will change the tcpm_next pointer of the element that will be deleted
> and so a thread iterating over the cache's entries while holding the
> RCU-lock might get "redirected" to this tmlist.
>
> This patch fixes this, by reverting back to the old behavior prior to
> bbf852b96ebdc6d1, which means that we simply change the tcpm_next
> pointer of the previous element (pp) to jump over the one we are
> deleting.
> The difference is that we call kfree_rcu() directly on the cache entry,
> which allows us to delete multiple entries from the list.
>
> Fixes: bbf852b96ebdc6d1 (tcp: metrics: Delete all entries matching a certain destination)
> Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
Looks good, applied, thank you.
^ permalink raw reply
* Re: [PATCH] ip_tunnel: clear IPCB in ip_tunnel_xmit() in case dst_link_failure() is called
From: David Miller @ 2014-01-23 5:24 UTC (permalink / raw)
To: pshelar; +Cc: duanj.fnst, daniel.petre, edumazet, netdev
In-Reply-To: <CALnjE+qFPoK_0EnZ_TWe1sx4s166iSjAH_+1r2uENeS4aRBckQ@mail.gmail.com>
From: Pravin Shelar <pshelar@nicira.com>
Date: Tue, 21 Jan 2014 10:33:36 -0800
> Can you clear IPCB in error path for dst_link_failure(). So that we do
> not clear it twice in fast path.
Indeed, please add this memset() before the dst_link_failure() call.
Thanks.
^ permalink raw reply
* [PATCH v2 2/2] net/cxgb4: Don't retrieve stats during recovery
From: Gavin Shan @ 2014-01-23 4:27 UTC (permalink / raw)
To: netdev; +Cc: dm, ben, davem, Gavin Shan
In-Reply-To: <1390451256-11486-1-git-send-email-shangw@linux.vnet.ibm.com>
We possibly retrieve the adapter's statistics during EEH recovery
and that should be disallowed. Otherwise, it would possibly incur
replicate EEH error and EEH recovery is going to fail eventually.
The patch reuses statistics lock and checks net_device is attached
before going to retrieve statistics, so that the problem can be
avoided.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index c8eafbf..139a704 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -4288,7 +4288,15 @@ static struct rtnl_link_stats64 *cxgb_get_stats(struct net_device *dev,
struct port_info *p = netdev_priv(dev);
struct adapter *adapter = p->adapter;
+ /* Block retrieving statistics during EEH error
+ * recovery. Otherwise, the recovery might fail
+ * and the PCI device will be removed permanently
+ */
spin_lock(&adapter->stats_lock);
+ if (!netif_device_present(dev)) {
+ spin_unlock(&adapter->stats_lock);
+ return ns;
+ }
t4_get_port_stats(adapter, p->tx_chan, &stats);
spin_unlock(&adapter->stats_lock);
@@ -5496,12 +5504,14 @@ static pci_ers_result_t eeh_err_detected(struct pci_dev *pdev,
rtnl_lock();
adap->flags &= ~FW_OK;
notify_ulds(adap, CXGB4_STATE_START_RECOVERY);
+ spin_lock(&adap->stats_lock);
for_each_port(adap, i) {
struct net_device *dev = adap->port[i];
netif_device_detach(dev);
netif_carrier_off(dev);
}
+ spin_unlock(&adap->stats_lock);
if (adap->flags & FULL_INIT_DONE)
cxgb_down(adap);
rtnl_unlock();
--
1.7.10.4
^ permalink raw reply related
* [PATCH v2 1/2] net/cxgb4: Avoid disabling PCI device for towice
From: Gavin Shan @ 2014-01-23 4:27 UTC (permalink / raw)
To: netdev; +Cc: dm, ben, davem, Gavin Shan
If we have EEH error happens to the adapter and we have to remove
it from the system for some reasons (e.g. more than 5 EEH errors
detected from the device in last hour), the adapter will be disabled
for towice separately by eeh_err_detected() and remove_one(), which
will incur following unexpected backtrace. The patch tries to avoid
it.
WARNING: at drivers/pci/pci.c:1431
CPU: 12 PID: 121 Comm: eehd Not tainted 3.13.0-rc7+ #1
task: c0000001823a3780 ti: c00000018240c000 task.ti: c00000018240c000
NIP: c0000000003c1e40 LR: c0000000003c1e3c CTR: 0000000001764c5c
REGS: c00000018240f470 TRAP: 0700 Not tainted (3.13.0-rc7+)
MSR: 8000000000029032 <SF,EE,ME,IR,DR,RI> CR: 28000024 XER: 00000004
CFAR: c000000000706528 SOFTE: 1
GPR00: c0000000003c1e3c c00000018240f6f0 c0000000010fe1f8 0000000000000035
GPR04: 0000000000000000 0000000000000000 00000000003ae509 0000000000000000
GPR08: 000000000000346f 0000000000000000 0000000000000000 0000000000003fef
GPR12: 0000000028000022 c00000000ec93000 c0000000000c11b0 c000000184ac3e40
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 0000000000000000 c0000000009398d8 c00000000101f9c0 c0000001860ae000
GPR28: c000000182ba0000 00000000000001f0 c0000001860ae6f8 c0000001860ae000
NIP [c0000000003c1e40] .pci_disable_device+0xd0/0xf0
LR [c0000000003c1e3c] .pci_disable_device+0xcc/0xf0
Call Trace:
[c0000000003c1e3c] .pci_disable_device+0xcc/0xf0 (unreliable)
[d0000000073881c4] .remove_one+0x174/0x320 [cxgb4]
[c0000000003c57e0] .pci_device_remove+0x60/0x100
[c00000000046396c] .__device_release_driver+0x9c/0x120
[c000000000463a20] .device_release_driver+0x30/0x60
[c0000000003bcdb4] .pci_stop_bus_device+0x94/0xd0
[c0000000003bcf48] .pci_stop_and_remove_bus_device+0x18/0x30
[c00000000003f548] .pcibios_remove_pci_devices+0xa8/0x140
[c000000000035c00] .eeh_handle_normal_event+0xa0/0x3c0
[c000000000035f50] .eeh_handle_event+0x30/0x2b0
[c0000000000362c4] .eeh_event_handler+0xf4/0x1b0
[c0000000000c12b8] .kthread+0x108/0x130
[c00000000000a168] .ret_from_kernel_thread+0x5c/0x74
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 5 +++--
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 23 ++++++++++++++++++-----
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index 56e0415..b4ca0a1 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -387,8 +387,9 @@ struct work_struct;
enum { /* adapter flags */
FULL_INIT_DONE = (1 << 0),
- USING_MSI = (1 << 1),
- USING_MSIX = (1 << 2),
+ DEV_ENABLED = (1 << 1),
+ USING_MSI = (1 << 2),
+ USING_MSIX = (1 << 3),
FW_OK = (1 << 4),
RSS_TNLALLLOOKUP = (1 << 5),
USING_SOFT_PARAMS = (1 << 6),
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index fff02ed..c8eafbf 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5505,7 +5505,10 @@ static pci_ers_result_t eeh_err_detected(struct pci_dev *pdev,
if (adap->flags & FULL_INIT_DONE)
cxgb_down(adap);
rtnl_unlock();
- pci_disable_device(pdev);
+ if ((adap->flags & DEV_ENABLED)) {
+ pci_disable_device(pdev);
+ adap->flags &= ~DEV_ENABLED;
+ }
out: return state == pci_channel_io_perm_failure ?
PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET;
}
@@ -5522,9 +5525,13 @@ static pci_ers_result_t eeh_slot_reset(struct pci_dev *pdev)
return PCI_ERS_RESULT_RECOVERED;
}
- if (pci_enable_device(pdev)) {
- dev_err(&pdev->dev, "cannot reenable PCI device after reset\n");
- return PCI_ERS_RESULT_DISCONNECT;
+ if (!(adap->flags & DEV_ENABLED)) {
+ if (pci_enable_device(pdev)) {
+ dev_err(&pdev->dev, "Cannot reenable PCI "
+ "device after reset\n");
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+ adap->flags |= DEV_ENABLED;
}
pci_set_master(pdev);
@@ -5910,6 +5917,9 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
goto out_disable_device;
}
+ /* PCI device has been enabled */
+ adapter->flags |= DEV_ENABLED;
+
adapter->regs = pci_ioremap_bar(pdev, 0);
if (!adapter->regs) {
dev_err(&pdev->dev, "cannot map device registers\n");
@@ -6145,7 +6155,10 @@ static void remove_one(struct pci_dev *pdev)
iounmap(adapter->bar2);
kfree(adapter);
pci_disable_pcie_error_reporting(pdev);
- pci_disable_device(pdev);
+ if ((adapter->flags & DEV_ENABLED)) {
+ pci_disable_device(pdev);
+ adapter->flags &= ~DEV_ENABLED;
+ }
pci_release_regions(pdev);
} else
pci_release_regions(pdev);
--
1.7.10.4
^ permalink raw reply related
* [PATCH] team: Don't allow team devices to change network namespaces.
From: Chen Weilong @ 2014-01-23 3:07 UTC (permalink / raw)
To: jiri, davem; +Cc: netdev
From: Weilong Chen <chenweilong@huawei.com>
Like bonding, team as netdevice doesn't cross netns boundaries.
Team ports and team itself live in same netns.
Signed-off-by: Weilong Chen <chenweilong@huawei.com>
---
drivers/net/team/team.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index dff24e3..2840742 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -2034,6 +2034,10 @@ static void team_setup(struct net_device *dev)
dev->features |= NETIF_F_LLTX;
dev->features |= NETIF_F_GRO;
+
+ /* Don't allow team devices to change network namespaces. */
+ dev->features |= NETIF_F_NETNS_LOCAL;
+
dev->hw_features = TEAM_VLAN_FEATURES |
NETIF_F_HW_VLAN_CTAG_TX |
NETIF_F_HW_VLAN_CTAG_RX |
--
1.7.12
^ permalink raw reply related
* Re: Linux kernel patch: elide fib_validate_source() completely when possible - bad side effect?
From: Hannes Frederic Sowa @ 2014-01-23 3:03 UTC (permalink / raw)
To: David Miller; +Cc: ja, gregory.hoggarth, netdev
In-Reply-To: <20140122.185305.1125981867482223830.davem@davemloft.net>
On Wed, Jan 22, 2014 at 06:53:05PM -0800, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Thu, 23 Jan 2014 03:02:24 +0100
>
> > On Wed, Jan 22, 2014 at 11:04:52PM +0200, Julian Anastasov wrote:
> >> I don't remember someone mentioning about such
> >> side-effect, I guess it is overlooked. IMHO, it is not a good
> >> reason to restore the old behavior. Lets see other opinions.
> >
> > A routing lookup to check for broadcast is much too expansive, I agree. But
> > can't we just check skb->pkt_type == PACKET_BROADCAST?
>
> We can't universally know what a broadcast is, because the prefix of
> every network is a local piece of information.
>
> We might not have the route necessary to know the source address is
> a broadcast if the packet came through more than 1 hop already.
But in this case we only need those local pieces.
E.g. we register all local registered broadcast addresses in a structure
like inet_addr_lst so we only need to check if the packet would leave
this host with a broadcast hardware address. If the packet is forwarded
the router must do the same check as only it knows the local broadcast
addresses. I hope this is correct. ;)
^ permalink raw reply
* Re: Linux kernel patch: elide fib_validate_source() completely when possible - bad side effect?
From: David Miller @ 2014-01-23 2:53 UTC (permalink / raw)
To: hannes; +Cc: ja, gregory.hoggarth, netdev
In-Reply-To: <20140123020224.GG7269@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 23 Jan 2014 03:02:24 +0100
> On Wed, Jan 22, 2014 at 11:04:52PM +0200, Julian Anastasov wrote:
>> I don't remember someone mentioning about such
>> side-effect, I guess it is overlooked. IMHO, it is not a good
>> reason to restore the old behavior. Lets see other opinions.
>
> A routing lookup to check for broadcast is much too expansive, I agree. But
> can't we just check skb->pkt_type == PACKET_BROADCAST?
We can't universally know what a broadcast is, because the prefix of
every network is a local piece of information.
We might not have the route necessary to know the source address is
a broadcast if the packet came through more than 1 hop already.
^ permalink raw reply
* Re: [PATCH net-next 1/3] bonding: Set the correct value to fail_over_mac at enslavement
From: Ding Tianhong @ 2014-01-23 2:50 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: Veaceslav Falico, David S. Miller, Netdev, Andy Gospodarek
In-Reply-To: <26505.1390423887@death.nxdomain>
On 2014/1/23 4:51, Jay Vosburgh wrote:
> Ding Tianhong <dingtianhong@huawei.com> wrote:
>
>> If the new slave don't support setting the MAC address, there are
>> two ways to handle this situation:
>>
>> 1). If the new slave is the first slave, set bond to the new slave's
>> MAC address, if the mode is active-backup, set fail_over_mac to
>> active, otherwise set fail_over_mac to none.
>
> This should be "if the mode is active-backup, set fail_over_mac
> to active, otherwise do not change fail_over_mac." Setting to none here
> would undo any setting of fail_over_mac that the user had set prior to
> adding the first slave.
>
I thought about this question for a long time, I still can not think clearly.
ex: the first slave did not support setting MAC, and mode is RR, fail_over_mac=1(user set it
when load the driver), so the after the enslavement, the fail_over_mac is still 1,
then the second slave added, if the second slave did not support setting MAC, it will pass
the check and go ahead, until the dev_set_mac_address(), and return error, so
I think set it to none may cause the second slave return early, but no substance logic change.
I will modify this place as your opinion.
>> 2). If the new slave is not the first slave and the fail_over_mac is
>> active, it means that the slave could work normally in active-backup
>> mode, otherwise if the fail_over_mac is none, the slave could not
>> work normally for no active-backup mode, so bond could not ensalve
>> the new dev.
>
> This (#2) is not a code change, correct? You're just restating
> the existing behavior of the code, right?
>
> Also, I don't see where this patch set updates the slave removal
> processing where the slave's original MAC is restored. At present, this
> is done by a test against fail_over_mac, but should be tested against
> the mode and fail_over_mac.
>
> My comment to the prior version of this patchset, again:
>
> The correct way to fix this in general is to permit setting an
> option at any time, but only have it take effect in active-backup mode.
> This minimizes ordering requirements when setting options.
>
> I would instead modify the bond enslave and removal processing
> to check the mode in addition to fail_over_mac when setting a slave's
> MAC during enslavement. The change active slave processing already only
> calls the fail_over_mac function when in active-backup mode. This
> should also be a simpler change set.
>
> These comments still apply to this version of the patchset.
>
> -J
>
OK.
Regards
Ding
>> Cc: Jay Vosburgh <fubar@us.ibm.com>
>> Cc: Veaceslav Falico <vfalico@redhat.com>
>> Cc: Andy Gospodarek <andy@greyhouse.net>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_main.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 3220b48..598f100 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -1334,9 +1334,17 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>>
>> if (slave_ops->ndo_set_mac_address == NULL) {
>> if (!bond_has_slaves(bond)) {
>> - pr_warning("%s: Warning: The first slave device specified does not support setting the MAC address. Setting fail_over_mac to active.",
>> + pr_warning("%s: Warning: The first slave device specified does not support setting the MAC address.\n",
>> bond_dev->name);
>> - bond->params.fail_over_mac = BOND_FOM_ACTIVE;
>> + if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
>> + bond->params.fail_over_mac = BOND_FOM_ACTIVE;
>> + pr_warning("%s: Setting fail_over_mac to active for active-backup mode.\n",
>> + bond_dev->name);
>> + } else {
>> + bond->params.fail_over_mac = BOND_FOM_NONE;
>> + pr_warning("%s: Setting fail_over_mac to none for no active-backup modes",
>> + bond_dev->name);
>> + }
>> } else if (bond->params.fail_over_mac != BOND_FOM_ACTIVE) {
>> pr_err("%s: Error: The slave device specified does not support setting the MAC address, but fail_over_mac is not set to active.\n",
>> bond_dev->name);
>> --
>> 1.8.0
>
> ---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>
>
> .
>
^ permalink raw reply
* RE: [PATCH 2/2] net/cxgb4: Don't retrieve stats during recovery
From: Dimitrios Michailidis @ 2014-01-23 2:17 UTC (permalink / raw)
To: Gavin Shan; +Cc: netdev@vger.kernel.org
In-Reply-To: <20140123013134.GA5947@shangw.(null)>
On Wed, Jan 22, 2014 at 5:32 PM, Gavin Shan wrote:
> >On Mon, Jan 20, 2014 at 02:05:18PM -0800, Dimitris Michailidis wrote:
> >>On 01/19/2014 07:05 PM, Gavin Shan wrote:
> >>>We possiblly retrieve the adapter's statistics during EEH recovery
> >>>and that should be disallowed. Otherwise, it would possibly incur
> >>>replicate EEH error and EEH recovery is going to fail eventually.
> >>>The patch checks if the PCI device is off-line before statistic
> >>>retrieval.
> >>
> >>The net_devices are detached during EEH so I think
> >>netif_device_present is a better check than pci_channel_offline. I
> >>am not sure such a test should be left to each driver though. If you
> >>do end up putting it in the driver it needs better synchronization
> >>with the EEH handlers as Ben mentioned.
> >>
> >
> >Ok. I agree that netif_device_present() is better since the statistics
> >is more net_device specific (other than pci_dev). And it's more accurate
> >to use netif_device_present() based on what we have:
> >
> > pci_channel_offline() ----------+
> > eeh_err_detected() |
> > !netif_device_present() --+-----+
> > <EEH recovery> | |
> > !pci_channel_offline() ----------+ |
> > eeh_slot_reset() |
> > eeh_resume() |
> > netif_device_present() --------+
> >
> >For the syncrhonization, I think we can just reuse the "adap->stats_lock".
> >Something like this:
> >
> >static pci_ers_result_t eeh_err_detected(struct pci_dev *pdev,
> > pci_channel_state_t state)
> >{
> > :
> > spin_lock(&adap->stats_lock);
> > for_each_port(adap, i) {
> > struct net_device *dev = adap->port[i];
> >
> > netif_device_detach(dev);
> > netif_carrier_off(dev);
> > }
> > spin_unlock(&adap->stats_lock);
> > :
> >}
> >
> >static void eeh_resume(struct pci_dev *pdev)
> >{
> > :
> > spin_lock(&adap->stats_lock);
> > for_each_port(adap, i) {
> > struct net_device *dev = adap->port[i];
> >
> > if (netif_running(dev)) {
> > link_start(dev);
> > cxgb_set_rxmode(dev);
> > }
> > netif_device_attach(dev);
> > }
> > spin_unlock(&adap->stats_lock);
> > :
> >}
Both link_start and cxgb_set_rxmode here issue blocking commands to FW, these two cannot be under a spinlock. In fact I don't think you need locking here at all. The devices can be attached asynchronously relative to the stats code, we don't care if it races. On detach it matters but not here.
> >static struct rtnl_link_stats64 *cxgb_get_stats(struct net_device *dev,
> > struct rtnl_link_stats64 *ns)
> >{
> > :
> > spin_lock(&adapter->stats_lock);
> > if (!netif_device_present(dev)) {
> > spin_unlock(&adapter->stats_lock);
> > return ns;
> > }
> > t4_get_port_stats(adapter, p->tx_chan, &stats);
> > spin_unlock(&adapter->stats_lock);
> > :
> >}
> >
>
> Dimitris, Any more comments on this? :-)
Just the above. Thanks.
> If you think it's fine, I'm going to change it like this and send
> out "v2".
>
> Thanks,
> Gavin
>
> >>>
> >>>Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> >>>---
> >>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 11 +++++++++++
> >>> 1 file changed, 11 insertions(+)
> >>>
> >>>diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> >>>index c8eafbf..b0e72fb 100644
> >>>--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> >>>+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> >>>@@ -4288,6 +4288,17 @@ static struct rtnl_link_stats64
> *cxgb_get_stats(struct net_device *dev,
> >>> struct port_info *p = netdev_priv(dev);
> >>> struct adapter *adapter = p->adapter;
> >>>
> >>>+ /*
> >>>+ * We possibly retrieve the statistics while the PCI
> >>>+ * device is off-line. That would cause the recovery
> >>>+ * on off-lined PCI device going to fail. So it's
> >>>+ * reasonable to block it during the recovery period.
> >>>+ */
> >>>+ if (pci_channel_offline(adapter->pdev)) {
> >>>+ memset(ns, 0, sizeof(*ns));
> >>>+ return ns;
> >>>+ }
> >>>+
> >>> spin_lock(&adapter->stats_lock);
> >>> t4_get_port_stats(adapter, p->tx_chan, &stats);
> >>> spin_unlock(&adapter->stats_lock);
> >>>
^ permalink raw reply
* Re: [PATCH] bonding: Don't allow bond devices to change network namespaces.
From: chenweilong @ 2014-01-23 2:41 UTC (permalink / raw)
To: Jiri Pirko; +Cc: fubar, vfalico, andy, davem, netdev
In-Reply-To: <20140122094106.GB2814@minipsycho.orion>
On 2014/1/22 17:41, Jiri Pirko wrote:
> Wed, Jan 22, 2014 at 10:16:30AM CET, chenweilong@huawei.com wrote:
>> From: Weilong Chen <chenweilong@huawei.com>
>>
>> Like bridge, bonding as netdevice doesn't cross netns boundaries.
>>
>> Bonding ports and bonding itself live in same netns.
>
> I think should should be done for team as well. Openvs already
> has this. I believe that for vlans it is ok to change ns, right?
>
OK, I'll write a new patch for 'team'.
For vlan dev, I think it's more complex.
1.Move vlan dev to a new ns, its real dev is in old ns, vlan
can not work.
2.Move vlan real dev to a new ns, vlan dev is in old ns, vlan
can not work.
If someone want to move vlan and its real dev to new ns, he must
do 1) and 2) or 2) and 1), both way are breaking the vlan down. I
think move real dev to new ns, and add a new vlan dev will be better.
Maybe I need write another patch for 'vlan'.
>>
>> Signed-off-by: Weilong Chen <chenweilong@huawei.com>
>> ---
>> drivers/net/bonding/bond_main.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index f00dd45..897d153 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -3916,6 +3916,9 @@ void bond_setup(struct net_device *bond_dev)
>> * capable
>> */
>>
>> + /* Don't allow bond devices to change network namespaces. */
>> + bond_dev->features |= NETIF_F_NETNS_LOCAL;
>> +
>> bond_dev->hw_features = BOND_VLAN_FEATURES |
>> NETIF_F_HW_VLAN_CTAG_TX |
>> NETIF_F_HW_VLAN_CTAG_RX |
>> --
>> 1.7.12
>>
>>
>> --
>> 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: [PATCH net-next 3/3] bonding: cleanup some redundant code and wrong variables
From: Ding Tianhong @ 2014-01-23 2:32 UTC (permalink / raw)
To: Nikolay Aleksandrov, Jay Vosburgh, Veaceslav Falico,
Andy Gospodarek, David S. Miller, Netdev
In-Reply-To: <52DFD587.4070109@redhat.com>
On 2014/1/22 22:28, Nikolay Aleksandrov wrote:
> On 01/22/2014 10:22 AM, Ding Tianhong wrote:
>> The dev_set_mac_address() will check the dev->netdev_ops->ndo_set_mac_address,
>> so no need to check it in bond_set_mac_address().
>>
>> Fix the wrong variables for pr_err().
>>
>> Cc: Jay Vosburgh <fubar@us.ibm.com>
>> Cc: Veaceslav Falico <vfalico@redhat.com>
>> Cc: Andy Gospodarek <andy@greyhouse.net>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_main.c | 10 +---------
>> 1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index ce0f5c0..9d92f46 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -3509,15 +3509,7 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
>> */
>>
>> bond_for_each_slave(bond, slave, iter) {
>> - const struct net_device_ops *slave_ops = slave->dev->netdev_ops;
>> pr_debug("slave %p %s\n", slave, slave->dev->name);
>> -
>> - if (slave_ops->ndo_set_mac_address == NULL) {
>> - res = -EOPNOTSUPP;
>> - pr_debug("EOPNOTSUPP %s\n", slave->dev->name);
>> - goto unwind;
>> - }
>> -
>> res = dev_set_mac_address(slave->dev, addr);
>> if (res) {
>> /* TODO: consider downing the slave
>> @@ -4317,7 +4309,7 @@ static int bond_check_params(struct bond_params *params)
>> fail_over_mac_tbl);
>> if (fail_over_mac_value == -1) {
>> pr_err("Error: invalid fail_over_mac \"%s\"\n",
>> - arp_validate == NULL ? "NULL" : arp_validate);
>> + fail_over_mac == NULL ? "NULL" : fail_over_mac);
>> return -EINVAL;
>> }
> My option API changes include a fix for this which also removes the NULL check
> as it's already checked earlier if fail_over_mac is != NULL. If you'd like to
> fix this yourself I think you can also skip the NULL check when printing the error.
>
yes, it exist a long time, need to update. thanks.
> Cheers,
> Nik
>>
>>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH net-next 1/3] bonding: Set the correct value to fail_over_mac at enslavement
From: Ding Tianhong @ 2014-01-23 2:31 UTC (permalink / raw)
To: Nikolay Aleksandrov, Jay Vosburgh, Veaceslav Falico,
David S. Miller, Netdev, Andy Gospodarek
In-Reply-To: <52DFD3BB.5010708@redhat.com>
On 2014/1/22 22:20, Nikolay Aleksandrov wrote:
> On 01/22/2014 10:22 AM, Ding Tianhong wrote:
>> If the new slave don't support setting the MAC address, there are
>> two ways to handle this situation:
>>
>> 1). If the new slave is the first slave, set bond to the new slave's
>> MAC address, if the mode is active-backup, set fail_over_mac to
>> active, otherwise set fail_over_mac to none.
>> 2). If the new slave is not the first slave and the fail_over_mac is
>> active, it means that the slave could work normally in active-backup
>> mode, otherwise if the fail_over_mac is none, the slave could not
>> work normally for no active-backup mode, so bond could not ensalve
>> the new dev.
>>
>> Cc: Jay Vosburgh <fubar@us.ibm.com>
>> Cc: Veaceslav Falico <vfalico@redhat.com>
>> Cc: Andy Gospodarek <andy@greyhouse.net>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_main.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 3220b48..598f100 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -1334,9 +1334,17 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
>>
>> if (slave_ops->ndo_set_mac_address == NULL) {
>> if (!bond_has_slaves(bond)) {
>> - pr_warning("%s: Warning: The first slave device specified does not support setting the MAC address. Setting fail_over_mac to active.",
>> + pr_warning("%s: Warning: The first slave device specified does not support setting the MAC address.\n",
>> bond_dev->name);
>> - bond->params.fail_over_mac = BOND_FOM_ACTIVE;
>> + if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
>> + bond->params.fail_over_mac = BOND_FOM_ACTIVE;
>> + pr_warning("%s: Setting fail_over_mac to active for active-backup mode.\n",
>> + bond_dev->name);
>> + } else {
>> + bond->params.fail_over_mac = BOND_FOM_NONE;
>> + pr_warning("%s: Setting fail_over_mac to none for no active-backup modes",
> At least make the warnings consistent: "... for no active-backup mode.\n".
> Also you might consider switching to pr_warn.
>
ok, thanks.
>> + bond_dev->name);
>> + }
>> } else if (bond->params.fail_over_mac != BOND_FOM_ACTIVE) {
>> pr_err("%s: Error: The slave device specified does not support setting the MAC address, but fail_over_mac is not set to active.\n",
>> bond_dev->name);
>>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH] SUNRPC: Allow one callback request to be received from two sk_buff
From: shaobingqing @ 2014-01-23 2:23 UTC (permalink / raw)
To: Trond Myklebust
Cc: Dr Fields James Bruce, Miller David S., linuxnfs, netdev,
Linux Kernel Mailing List
In-Reply-To: <1BFACA51-087E-4945-851A-FBF0F108604C@primarydata.com>
2014/1/21 Trond Myklebust <trond.myklebust@primarydata.com>:
>
> On Jan 21, 2014, at 3:08, shaobingqing <shaobingqing@bwstor.com.cn> wrote:
>
>> 2014/1/21 Trond Myklebust <trond.myklebust@primarydata.com>:
>>> On Mon, 2014-01-20 at 14:59 +0800, shaobingqing wrote:
>>>> In current code, there only one struct rpc_rqst is prealloced. If one
>>>> callback request is received from two sk_buff, the xprt_alloc_bc_request
>>>> would be execute two times with the same transport->xid. The first time
>>>> xprt_alloc_bc_request will alloc one struct rpc_rqst and the TCP_RCV_COPY_DATA
>>>> bit of transport->tcp_flags will not be cleared. The second time
>>>> xprt_alloc_bc_request could not alloc struct rpc_rqst any more and NULL
>>>> pointer will be returned, then xprt_force_disconnect occur. I think one
>>>> callback request can be allowed to be received from two sk_buff.
>>>>
>>>> Signed-off-by: shaobingqing <shaobingqing@bwstor.com.cn>
>>>> ---
>>>> net/sunrpc/xprtsock.c | 11 +++++++++--
>>>> 1 files changed, 9 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
>>>> index ee03d35..606950d 100644
>>>> --- a/net/sunrpc/xprtsock.c
>>>> +++ b/net/sunrpc/xprtsock.c
>>>> @@ -1271,8 +1271,13 @@ static inline int xs_tcp_read_callback(struct rpc_xprt *xprt,
>>>> struct sock_xprt *transport =
>>>> container_of(xprt, struct sock_xprt, xprt);
>>>> struct rpc_rqst *req;
>>>> + static struct rpc_rqst *req_partial;
>>>> +
>>>> + if (req_partial == NULL)
>>>> + req = xprt_alloc_bc_request(xprt);
>>>> + else if (req_partial->rq_xid == transport->tcp_xid)
>>>> + req = req_partial;
>>>
>>> What happens here if req_partial->rq_xid != transport->tcp_xid? AFAICS,
>>> req will be undefined. Either way, you cannot use a static variable for
>>> storage here: that isn't re-entrant.
>>
>> Because metadata sever only have one slot for backchannel request,
>> req_partial->rq_xid == transport->tcp_xid always happens, if the callback
>> request just being splited in two sk_buffs. But req_partial->rq_xid !=
>> transport->tcp_xid may also happens in some special cases, such as
>> retransmission occurs?
>
> If the server retransmits, then it is broken. The NFSv4.1 protocol does not allow it to retransmit unless the connection breaks.
What I am saying above is bogus. As far as I can see, If one callback
request is splitted into two sk_buffs, the function
xs_tcp_read_callback will be called two times with the same rpc_xprt
and the same xid. If between the two calls there is
another call with the same rpc_xprt, but different xid, we consider it
is another callback request from the same server, in
the condition that there is no retransmission in our enviorenment. But
this might not happen because there is only one
callback slot in each server.
>
>> If one callback request is splited in two sk_buffs, xs_tcp_read_callback
>> will be execute two times. The req_partial should be a static variable,
>> because the second execution of xs_tcp_read_callback should use
>> the rpc_rqst allocated for the first execution, which saves information
>> copies from the first sk_buff.
>
> No! This is a multi-threaded/process environment which can support multiple connection. It is a bug to use a static variable.
I think I have misunderstood the question. Here a static variable can
not be used. Perhaps, we should define a variable for each
rpc_client (or rpc xprt).
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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: Linux kernel patch: elide fib_validate_source() completely when possible - bad side effect?
From: Hannes Frederic Sowa @ 2014-01-23 2:06 UTC (permalink / raw)
To: Julian Anastasov, gregory hoggarth, netdev
In-Reply-To: <20140123020224.GG7269@order.stressinduktion.org>
On Thu, Jan 23, 2014 at 03:02:24AM +0100, Hannes Frederic Sowa wrote:
> On Wed, Jan 22, 2014 at 11:04:52PM +0200, Julian Anastasov wrote:
> > I don't remember someone mentioning about such
> > side-effect, I guess it is overlooked. IMHO, it is not a good
> > reason to restore the old behavior. Lets see other opinions.
>
> A routing lookup to check for broadcast is much too expansive, I agree. But
> can't we just check skb->pkt_type == PACKET_BROADCAST?
Ah, sorry, other way around, we need to check the source address. Maybe a
small hash table to look for currently installed broadcast addresses?
Greetings,
Hannes
^ permalink raw reply
* Re: Linux kernel patch: elide fib_validate_source() completely when possible - bad side effect?
From: Hannes Frederic Sowa @ 2014-01-23 2:02 UTC (permalink / raw)
To: Julian Anastasov; +Cc: gregory hoggarth, netdev
In-Reply-To: <alpine.LFD.2.11.1401222222010.1855@ja.home.ssi.bg>
On Wed, Jan 22, 2014 at 11:04:52PM +0200, Julian Anastasov wrote:
> I don't remember someone mentioning about such
> side-effect, I guess it is overlooked. IMHO, it is not a good
> reason to restore the old behavior. Lets see other opinions.
A routing lookup to check for broadcast is much too expansive, I agree. But
can't we just check skb->pkt_type == PACKET_BROADCAST?
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH 0/4] ipv4: small set of fixes
From: David Miller @ 2014-01-23 1:54 UTC (permalink / raw)
To: popovich_sergei; +Cc: netdev
In-Reply-To: <cover.1390304505.git.popovich_sergei@mail.ru>
From: Sergey Popovich <popovich_sergei@mail.ru>
Date: Tue, 21 Jan 2014 13:48:47 +0200
> In this patch series I present my attempt to address
> some issues in IPv4 implementation.
>
> Please take time to review my patches.
You're going to have to repost these with the adjustments I've
asked for.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox