* Re: [PATCH net-next 4/7] sfc: Add support for IEEE-1588 PTP
From: Richard Cochran @ 2012-07-20 6:11 UTC (permalink / raw)
To: Andrew Jackson; +Cc: Ben Hutchings, David Miller, netdev, linux-net-drivers
In-Reply-To: <50083059.7070700@solarflare.com>
On Thu, Jul 19, 2012 at 05:05:45PM +0100, Andrew Jackson wrote:
> On 19/07/2012 15:37, Ben Hutchings wrote:
> >On Thu, 2012-07-19 at 16:25 +0200, Richard Cochran wrote:
> >>
> >>I am trying to purge the whole SYS thing (only blackfin is left)
> >>because there is a much better way to go about this, namely
> >>synchronizing the system time to the PHC time via an internal PPS
> >>signal.
> >
> >Andrew, would that work for us?
>
> I don't think so for the reason that Stu has pointed out (failed
> assumption).
>
> The NIC's clock isn't directly accessible by the host from the PCIe
> bus and is "behind" the MC. Even when we process PPS events, we need
> a reliable way of determining the relationship between the two
> clocks (system <> NIC). We're trying to get that as accurately as we
> can but we know that some measurements will be incorrect/out of
> bounds because of loading on the system.
There is a better way to filter out bad measurements than to hard code
some black magic into the driver. Instead, just offer a PPS, and then
we can comfortably (and flexibly) filter using a user space program.
Thanks,
Richard
^ permalink raw reply
* [ethtool 1/2] ethtool.h: implement new MDI-X set defines
From: Jeff Kirsher @ 2012-07-20 6:25 UTC (permalink / raw)
To: bhutchings; +Cc: Jesse Brandeburg, netdev, davem, Jeff Kirsher
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
These changes implement the kernel side of the interface
for allowing drivers to set MDI-X state on twisted pair.
Changes implemented as suggested by Ben Hutchings, thanks Ben!
see ethtool patches titled:
ethtool: allow setting MDI-X state
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
ethtool-copy.h | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/ethtool-copy.h b/ethtool-copy.h
index 0e90e9b..6625eff 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -42,8 +42,10 @@ struct ethtool_cmd {
* bits) in Mbps. Please use
* ethtool_cmd_speed()/_set() to
* access it */
- __u8 eth_tp_mdix;
- __u8 reserved2;
+ __u8 eth_tp_mdix; /* twisted pair MDI-X status */
+ __u8 eth_tp_mdix_ctrl; /* twisted pair MDI-X control, when set,
+ * link should be renegotiated if necessary
+ */
__u32 lp_advertising; /* Features the link partner advertises */
__u32 reserved[2];
};
@@ -989,10 +991,13 @@ enum ethtool_sfeatures_retval_bits {
#define AUTONEG_DISABLE 0x00
#define AUTONEG_ENABLE 0x01
-/* Mode MDI or MDI-X */
-#define ETH_TP_MDI_INVALID 0x00
-#define ETH_TP_MDI 0x01
-#define ETH_TP_MDI_X 0x02
+/* MDI or MDI-X status/control - if MDI/MDI_X/AUTO is set then
+ * the driver is required to renegotiate link
+ */
+#define ETH_TP_MDI_INVALID 0x00 /* status: unknown; control: unsupported */
+#define ETH_TP_MDI 0x01 /* status: MDI; control: force MDI */
+#define ETH_TP_MDI_X 0x02 /* status: MDI-X; control: force MDI-X */
+#define ETH_TP_MDI_AUTO 0x03 /* control: auto-select */
/* Wake-On-Lan options. */
#define WAKE_PHY (1 << 0)
--
1.7.10.4
^ permalink raw reply related
* [ethtool 2/2] ethtool: allow setting MDI-X state
From: Jeff Kirsher @ 2012-07-20 6:25 UTC (permalink / raw)
To: bhutchings; +Cc: Jesse Brandeburg, netdev, davem, Jeff Kirsher
In-Reply-To: <1342765524-29711-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
A bit ago ethtool added support for reading MDI-X state, this
patch finishes the implementation, adding the complementary write
command.
Add support to ethtool for controlling the MDI-X (crossover)
state of a network port. Most adapters correctly negotiate
MDI-X, but some ill-behaved switches have trouble and end up
picking the wrong MDI setting, which results in complete loss of
link. Usually this error condition can be observed when multiple
ethtool -r ethX are required before link is achieved.
This patch allows the user to override the normal "auto" setting
and force the crossover state to on or off.
The set will fail if the driver doesn't support the get, as
suggested by Ben Hutchings. Unfortunately there is no way to
tell the set isn't supported *if* the driver supports the get,
set just silently fails.
# ./ethtool -s p1p1 mdix off
setting MDI not supported
usage is ethtool -s eth0 mdix [auto|on|off]
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
ethtool.8.in | 8 ++++++++
ethtool.c | 26 ++++++++++++++++++++++++++
2 files changed, 34 insertions(+)
diff --git a/ethtool.8.in b/ethtool.8.in
index ff3255b..44943a2 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -229,6 +229,7 @@ ethtool \- query or control network driver and hardware settings
.BI speed \ N
.B2 duplex half full
.B4 port tp aui bnc mii fibre
+.B3 mdix auto on off
.B2 autoneg on off
.BN advertise
.BN phyad
@@ -521,6 +522,13 @@ Sets full or half duplex mode.
.A4 port tp aui bnc mii fibre
Selects device port.
.TP
+.A3 mdix auto on off
+Selects MDI-X mode for port. May be used to override the automatic detection
+feature of most adapters. Auto means automatic detection of MDI status, on
+forces MDI-X (crossover) mode, while off means MDI (straight through) mode.
+The driver should guarantee that this command takes effect immediately, and
+if necessary may reset the link to cause the change to take effect.
+.TP
.A2 autoneg on off
Specifies whether autonegotiation should be enabled. Autonegotiation
is enabled by default, but in some network devices may have trouble
diff --git a/ethtool.c b/ethtool.c
index e573357..9effc60 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -18,6 +18,8 @@
* Rx Network Flow Control configuration support <santwona.behera@sun.com>
* Various features by Ben Hutchings <bhutchings@solarflare.com>;
* Copyright 2009, 2010 Solarflare Communications
+ * MDI-X set support by Jesse Brandeburg <jesse.brandeburg@intel.com>
+ * Copyright 2012 Intel Corporation
*
* TODO:
* * show settings for all devices
@@ -2199,6 +2201,7 @@ static int do_sset(struct cmd_context *ctx)
int speed_wanted = -1;
int duplex_wanted = -1;
int port_wanted = -1;
+ int mdix_wanted = -1;
int autoneg_wanted = -1;
int phyad_wanted = -1;
int xcvr_wanted = -1;
@@ -2259,6 +2262,19 @@ static int do_sset(struct cmd_context *ctx)
port_wanted = PORT_FIBRE;
else
exit_bad_args();
+ } else if (!strcmp(argp[i], "mdix")) {
+ gset_changed = 1;
+ i += 1;
+ if (i >= argc)
+ exit_bad_args();
+ if (!strcmp(argp[i], "auto"))
+ mdix_wanted = ETH_TP_MDI_AUTO;
+ else if (!strcmp(argp[i], "on"))
+ mdix_wanted = ETH_TP_MDI_X;
+ else if (!strcmp(argp[i], "off"))
+ mdix_wanted = ETH_TP_MDI;
+ else
+ exit_bad_args();
} else if (!strcmp(argp[i], "autoneg")) {
i += 1;
if (i >= argc)
@@ -2380,6 +2396,13 @@ static int do_sset(struct cmd_context *ctx)
ecmd.duplex = duplex_wanted;
if (port_wanted != -1)
ecmd.port = port_wanted;
+ if (mdix_wanted != -1) {
+ /* check driver supports MDI-X */
+ if (ecmd.eth_tp_mdix != ETH_TP_MDI_INVALID)
+ ecmd.eth_tp_mdix_ctrl = mdix_wanted;
+ else
+ fprintf(stderr, "setting MDI not supported\n");
+ }
if (autoneg_wanted != -1)
ecmd.autoneg = autoneg_wanted;
if (phyad_wanted != -1)
@@ -2439,6 +2462,8 @@ static int do_sset(struct cmd_context *ctx)
fprintf(stderr, " not setting phy_address\n");
if (xcvr_wanted != -1)
fprintf(stderr, " not setting transceiver\n");
+ if (mdix_wanted != -1)
+ fprintf(stderr, " not setting mdix\n");
}
}
@@ -3604,6 +3629,7 @@ static const struct option {
" [ speed %d ]\n"
" [ duplex half|full ]\n"
" [ port tp|aui|bnc|mii|fibre ]\n"
+ " [ mdix auto|on|off ]\n"
" [ autoneg on|off ]\n"
" [ advertise %x ]\n"
" [ phyad %d ]\n"
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH net-next 4/7] sfc: Add support for IEEE-1588 PTP
From: Richard Cochran @ 2012-07-20 6:31 UTC (permalink / raw)
To: Stuart Hodgson
Cc: Ben Hutchings, David Miller, netdev, linux-net-drivers,
Andrew Jackson
In-Reply-To: <500827EF.208@solarflare.com>
On Thu, Jul 19, 2012 at 04:29:51PM +0100, Stuart Hodgson wrote:
> On 19/07/12 15:25, Richard Cochran wrote:
> > On Wed, Jul 18, 2012 at 07:21:33PM +0100, Ben Hutchings wrote:
> >> Add PTP IEEE-1588 support and make accesible via the PHC subsystem.
...
> >> +/* Process times received from MC.
> >> + *
> >> + * Extract times from returned results, and establish the minimum value
> >> + * seen. The minimum value represents the "best" possible time and events
> >> + * too much greater than this are rejected - the machine is, perhaps, too
> >> + * busy. A number of readings are taken so that, hopefully, at least one good
> >> + * synchronisation will be seen in the results.
> >> + */
> >
> > This code looks like it is trying to find the offset between two
> > clocks. Is there some reason why you cannot use <linux/timecompare.h>
> > to accomplish this?
>
> This is what the code is doing. <linux/timecompare.h> states
>
> "the assumption is that reading the source
> time is slow and involves equal time for sending the request and
> receiving the reply"
>
> While in our case event though it is slow we cannot guarantee the second
> assumption. The code above takes into account some of the particulars of the sfc
> hardware and gives us good results.
Fair enough, but then maybe a comment mentioning how timecompare is
unsuitable would be nice to have.
> > I am trying to purge the whole SYS thing (only blackfin is left)
> > because there is a much better way to go about this, namely
> > synchronizing the system time to the PHC time via an internal PPS
> > signal.
>
> This may be possible in future. But leads us to another problem
> where the PPS event that is generated by the PHC subsystem to the
> PPS subsystem is stamped with the current system_time. That may
> be fine for a PPS signal generated from an interrupt but not when
> the internal PPS event has implicit jitter from the handler/event_queue
> that we have in the driver.
Yes, both your driver and timecompare** suffer from the same
defect, namely that the synchronization times are determined by
network traffic, and not at some regular frequency like 1 Hz.
Why not just offer a 1 PPS signal using the standard method? If you
cannot produce a real interrupt for this, you can at least use a
synthetic one using a periodically scheduled work.
** Actually, timecompare is a bit better as it ensures that the
measurements occur no more than once per second.
> Calling pps_event directly does not seem possible with the current
> header/structure layout preventing access to ptp_clock->pps_source.
> Including ptp_private.h seems wrong.
I don't understand what the issue is here. Can't you just call
ptp_clock_event, like you already have...
> >> +static void efx_ptp_pps_worker(struct work_struct *work)
> >> +{
> >> + struct efx_ptp_data *ptp =
> >> + container_of(work, struct efx_ptp_data, pps_work);
> >> + struct efx_nic *efx = ptp->channel->efx;
> >> + struct timespec event_gen_time;
> >> + struct ptp_clock_event ptp_pps_evt;
> >> + ktime_t gen_time_host;
> >> +
> >> + if (efx_ptp_synchronize(efx, PTP_SYNC_ATTEMPTS))
> >> + return;
> >> +
> >> + gen_time_host = ktime_sub(ptp->mc_base_time,
> >> + ptp->host_base_time);
> >> + event_gen_time = ktime_to_timespec(gen_time_host);
> >> +
> >> + ptp_pps_evt.type = PTP_CLOCK_EXTTS;
> >> + ptp_pps_evt.timestamp = ktime_to_ns(gen_time_host);
> >> + ptp_clock_event(ptp->phc_clock, &ptp_pps_evt);
> >> +}
... here?
> >> +static int efx_phc_adjtime(struct ptp_clock_info *ptp, s64 delta)
> >> +{
You can set the time here somehow by doing, T' = T + offset, and so...
> >> +}
> >> +static int efx_phc_settime(struct ptp_clock_info *ptp,
> >> + const struct timespec *e_ts)
> >> +{
> >> + /* We must provide this function, but we cannot actually set the time */
> >
> > Huh? You can adjtime, so must be able to settime, too, right?
> >
> > If you have enough range in the RAW timestamp in the MC firmware (like
> > 64 bits of nanoseconds), and you allow settime, then you can spare the
> > system time synchronization code altogether.
> >
>
> You will have to elaborate further on this point.
... why can't you also just set the time?
Thanks,
Richard
^ permalink raw reply
* [trivial PATCH resend 0/2]
From: Jon Mason @ 2012-07-20 7:02 UTC (permalink / raw)
To: trivial; +Cc: netdev, linux-kernel
Resending original patch #1 and sending a revised patch #2 to netdev per
Jeff Kirsher's request. The rest of the original series of 7 is not
being resent.
^ permalink raw reply
* [trivial PATCH resend 1/2] ixgb: use PCI_VENDOR_ID_INTEL
From: Jon Mason @ 2012-07-20 7:02 UTC (permalink / raw)
To: trivial
Cc: netdev, linux-kernel, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
Alex Duyck, John Ronciak
In-Reply-To: <1342767729-17788-1-git-send-email-jdmason@kudzu.us>
Use PCI_VENDOR_ID_INTEL from pci_ids.h instead of creating its own
vendor ID #define.
Signed-off-by: Jon Mason <jdmason@kudzu.us>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Cc: Greg Rose <gregory.v.rose@intel.com>
Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Cc: Alex Duyck <alexander.h.duyck@intel.com>
Cc: John Ronciak <john.ronciak@intel.com>
---
drivers/net/ethernet/intel/ixgb/ixgb_hw.c | 5 +++--
drivers/net/ethernet/intel/ixgb/ixgb_ids.h | 5 -----
drivers/net/ethernet/intel/ixgb/ixgb_main.c | 10 +++++-----
3 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_hw.c b/drivers/net/ethernet/intel/ixgb/ixgb_hw.c
index 99b69ad..bf9a220 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_hw.c
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_hw.c
@@ -32,6 +32,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/pci_ids.h>
#include "ixgb_hw.h"
#include "ixgb_ids.h"
@@ -96,7 +97,7 @@ static u32 ixgb_mac_reset(struct ixgb_hw *hw)
ASSERT(!(ctrl_reg & IXGB_CTRL0_RST));
#endif
- if (hw->subsystem_vendor_id == SUN_SUBVENDOR_ID) {
+ if (hw->subsystem_vendor_id == PCI_VENDOR_ID_SUN) {
ctrl_reg = /* Enable interrupt from XFP and SerDes */
IXGB_CTRL1_GPI0_EN |
IXGB_CTRL1_SDP6_DIR |
@@ -271,7 +272,7 @@ ixgb_identify_phy(struct ixgb_hw *hw)
}
/* update phy type for sun specific board */
- if (hw->subsystem_vendor_id == SUN_SUBVENDOR_ID)
+ if (hw->subsystem_vendor_id == PCI_VENDOR_ID_SUN)
phy_type = ixgb_phy_type_bcm;
return phy_type;
diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_ids.h b/drivers/net/ethernet/intel/ixgb/ixgb_ids.h
index 2a58847..32c1b30 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_ids.h
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_ids.h
@@ -33,11 +33,6 @@
** The Device and Vendor IDs for 10 Gigabit MACs
**********************************************************************/
-#define INTEL_VENDOR_ID 0x8086
-#define INTEL_SUBVENDOR_ID 0x8086
-#define SUN_VENDOR_ID 0x108E
-#define SUN_SUBVENDOR_ID 0x108E
-
#define IXGB_DEVICE_ID_82597EX 0x1048
#define IXGB_DEVICE_ID_82597EX_SR 0x1A48
#define IXGB_DEVICE_ID_82597EX_LR 0x1B48
diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
index 5fce363..4e5a060 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
@@ -54,13 +54,13 @@ MODULE_PARM_DESC(copybreak,
* Class, Class Mask, private data (not used) }
*/
static DEFINE_PCI_DEVICE_TABLE(ixgb_pci_tbl) = {
- {INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX,
+ {PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- {INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX_CX4,
+ {PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX_CX4,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- {INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX_SR,
+ {PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX_SR,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
- {INTEL_VENDOR_ID, IXGB_DEVICE_ID_82597EX_LR,
+ {PCI_VENDOR_ID_INTEL, IXGB_DEVICE_ID_82597EX_LR,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
/* required last entry */
@@ -195,7 +195,7 @@ ixgb_irq_enable(struct ixgb_adapter *adapter)
{
u32 val = IXGB_INT_RXT0 | IXGB_INT_RXDMT0 |
IXGB_INT_TXDW | IXGB_INT_LSC;
- if (adapter->hw.subsystem_vendor_id == SUN_SUBVENDOR_ID)
+ if (adapter->hw.subsystem_vendor_id == PCI_VENDOR_ID_SUN)
val |= IXGB_INT_GPI0;
IXGB_WRITE_REG(&adapter->hw, IMS, val);
IXGB_WRITE_FLUSH(&adapter->hw);
--
1.7.9.5
^ permalink raw reply related
* [trivial PATCH resend 2/2] ixgbe: use PCI_VENDOR_ID_INTEL
From: Jon Mason @ 2012-07-20 7:02 UTC (permalink / raw)
To: trivial
Cc: netdev, linux-kernel, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
Alex Duyck, John Ronciak
In-Reply-To: <1342767729-17788-1-git-send-email-jdmason@kudzu.us>
Use PCI_VENDOR_ID_INTEL from pci_ids.h instead of creating its own
vendor ID #define.
Signed-off-by: Jon Mason <jdmason@kudzu.us>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Cc: Greg Rose <gregory.v.rose@intel.com>
Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Cc: Alex Duyck <alexander.h.duyck@intel.com>
Cc: John Ronciak <john.ronciak@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 8 ++++----
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 3 ---
3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 18ca3bc..8a3fafb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7516,11 +7516,11 @@ static pci_ers_result_t ixgbe_io_error_detected(struct pci_dev *pdev,
}
/* Find the pci device of the offending VF */
- vfdev = pci_get_device(IXGBE_INTEL_VENDOR_ID, device_id, NULL);
+ vfdev = pci_get_device(PCI_VENDOR_ID_INTEL, device_id, NULL);
while (vfdev) {
if (vfdev->devfn == (req_id & 0xFF))
break;
- vfdev = pci_get_device(IXGBE_INTEL_VENDOR_ID,
+ vfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
device_id, vfdev);
}
/*
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 2d971d1..28018d4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -65,13 +65,13 @@ static int ixgbe_find_enabled_vfs(struct ixgbe_adapter *adapter)
}
vf_devfn = pdev->devfn + 0x80;
- pvfdev = pci_get_device(IXGBE_INTEL_VENDOR_ID, device_id, NULL);
+ pvfdev = pci_get_device(PCI_VENDOR_ID_INTEL, device_id, NULL);
while (pvfdev) {
if (pvfdev->devfn == vf_devfn &&
(pvfdev->bus->number >= pdev->bus->number))
vfs_found++;
vf_devfn += 2;
- pvfdev = pci_get_device(IXGBE_INTEL_VENDOR_ID,
+ pvfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
device_id, pvfdev);
}
@@ -488,11 +488,11 @@ int ixgbe_vf_configuration(struct pci_dev *pdev, unsigned int event_mask)
break;
}
- pvfdev = pci_get_device(IXGBE_INTEL_VENDOR_ID, device_id, NULL);
+ pvfdev = pci_get_device(PCI_VENDOR_ID_INTEL, device_id, NULL);
while (pvfdev) {
if (pvfdev->devfn == thisvf_devfn)
break;
- pvfdev = pci_get_device(IXGBE_INTEL_VENDOR_ID,
+ pvfdev = pci_get_device(PCI_VENDOR_ID_INTEL,
device_id, pvfdev);
}
if (pvfdev)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index 204848d..c8d8040 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -32,9 +32,6 @@
#include <linux/mdio.h>
#include <linux/netdevice.h>
-/* Vendor ID */
-#define IXGBE_INTEL_VENDOR_ID 0x8086
-
/* Device IDs */
#define IXGBE_DEV_ID_82598 0x10B6
#define IXGBE_DEV_ID_82598_BX 0x1508
--
1.7.9.5
^ permalink raw reply related
* Re: [trivial PATCH resend 1/2] ixgb: use PCI_VENDOR_ID_INTEL
From: Jeff Kirsher @ 2012-07-20 7:11 UTC (permalink / raw)
To: Jon Mason
Cc: trivial, netdev, linux-kernel, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
Alex Duyck, John Ronciak
In-Reply-To: <1342767729-17788-2-git-send-email-jdmason@kudzu.us>
[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]
On Fri, 2012-07-20 at 00:02 -0700, Jon Mason wrote:
> Use PCI_VENDOR_ID_INTEL from pci_ids.h instead of creating its own
> vendor ID #define.
>
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Cc: Greg Rose <gregory.v.rose@intel.com>
> Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Cc: Alex Duyck <alexander.h.duyck@intel.com>
> Cc: John Ronciak <john.ronciak@intel.com>
> ---
> drivers/net/ethernet/intel/ixgb/ixgb_hw.c | 5 +++--
> drivers/net/ethernet/intel/ixgb/ixgb_ids.h | 5 -----
> drivers/net/ethernet/intel/ixgb/ixgb_main.c | 10 +++++-----
> 3 files changed, 8 insertions(+), 12 deletions(-)
Like I said before, I do not consider this patch "trivial" so it is
interesting that you submitted again as a trivial patch. At least you
CC'd netdev like I asked.
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [trivial PATCH resend 2/2] ixgbe: use PCI_VENDOR_ID_INTEL
From: Jeff Kirsher @ 2012-07-20 7:14 UTC (permalink / raw)
To: Jon Mason
Cc: trivial, netdev, linux-kernel, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
Alex Duyck, John Ronciak
In-Reply-To: <1342767729-17788-3-git-send-email-jdmason@kudzu.us>
[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]
On Fri, 2012-07-20 at 00:02 -0700, Jon Mason wrote:
> Use PCI_VENDOR_ID_INTEL from pci_ids.h instead of creating its own
> vendor ID #define.
>
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Cc: Greg Rose <gregory.v.rose@intel.com>
> Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Cc: Alex Duyck <alexander.h.duyck@intel.com>
> Cc: John Ronciak <john.ronciak@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 8 ++++----
> drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 3 ---
> 3 files changed, 6 insertions(+), 9 deletions(-)
Same goes for this patch as well (not being trivial).
I already have several patches submitted for Dave against ixgbe
currently and I am not sure if there would be any issues with this patch
applying on top of the currently submitted patches. I will verify this
applies cleanly with no issue to my current net-next tree before I send
my ACK.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* RE: [RFC] r8169 : why SG / TX checksum are default disabled
From: hayeswang @ 2012-07-20 7:14 UTC (permalink / raw)
To: 'David Miller', eric.dumazet; +Cc: romieu, netdev
In-Reply-To: <20120718.152405.1083396282134539674.davem@davemloft.net>
Francois Romieu <romieu@fr.zoreil.com>
> >> > A NETDEV_TX_OK return means we accepted the SKB, it
> doesn't look like
> >> > that's what you are doing in the skb_padto() failure path.
> >>
> >> ?
> >>
> >> - skb_padto fails
> >> (original skb is implicitely freed)
> >> - skb_padto returns error status (!= 0)
> >> - rtl8169_tso_csum returns false
> >> - start_xmit returns NETDEV_TX_OK.
> >>
> >> I'll search the missing "!" after some sleep if that's
> what you are talking
> >> about. Otherwise than that, I don't get it.
> >>
> >
> >
> > Yes, I believe your patch is fine.
> >
> > In fact many drivers dont account the error in their stats.
>
> My bad, I forgot that skb_padto() frees the SKB on failure.
>
I find that the total length field of IP header would be modified if the hw
checksum is enabled. Therefore, skb_padto + hw checksum wouldn't work. The
software checksum is necessary.
Best Regards,
Hayes
^ permalink raw reply
* Re: [trivial PATCH resend 2/2] ixgbe: use PCI_VENDOR_ID_INTEL
From: Jeff Kirsher @ 2012-07-20 7:17 UTC (permalink / raw)
To: Jon Mason
Cc: trivial, netdev, linux-kernel, Jesse Brandeburg, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
Alex Duyck, John Ronciak
In-Reply-To: <1342767729-17788-3-git-send-email-jdmason@kudzu.us>
[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]
On Fri, 2012-07-20 at 00:02 -0700, Jon Mason wrote:
> Use PCI_VENDOR_ID_INTEL from pci_ids.h instead of creating its own
> vendor ID #define.
>
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Cc: Greg Rose <gregory.v.rose@intel.com>
> Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Cc: Alex Duyck <alexander.h.duyck@intel.com>
> Cc: John Ronciak <john.ronciak@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 8 ++++----
> drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 3 ---
> 3 files changed, 6 insertions(+), 9 deletions(-)
This applies cleanly against my current net-next tree and looks fine.
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH net-next] tcp: Return bool instead of int where appropriate
From: Vijay Subramanian @ 2012-07-20 7:32 UTC (permalink / raw)
To: netdev; +Cc: davem, Vijay Subramanian
Applied to a set of static inline functions in tcp_input.c
Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
---
net/ipv4/tcp_input.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e67d685..21d7f8f 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2521,7 +2521,7 @@ static void tcp_cwnd_down(struct sock *sk, int flag)
/* Nothing was retransmitted or returned timestamp is less
* than timestamp of the first retransmission.
*/
-static inline int tcp_packet_delayed(const struct tcp_sock *tp)
+static inline bool tcp_packet_delayed(const struct tcp_sock *tp)
{
return !tp->retrans_stamp ||
(tp->rx_opt.saw_tstamp && tp->rx_opt.rcv_tsecr &&
@@ -2582,7 +2582,7 @@ static void tcp_undo_cwr(struct sock *sk, const bool undo_ssthresh)
tp->snd_cwnd_stamp = tcp_time_stamp;
}
-static inline int tcp_may_undo(const struct tcp_sock *tp)
+static inline bool tcp_may_undo(const struct tcp_sock *tp)
{
return tp->undo_marker && (!tp->undo_retrans || tcp_packet_delayed(tp));
}
@@ -3371,13 +3371,13 @@ static void tcp_ack_probe(struct sock *sk)
}
}
-static inline int tcp_ack_is_dubious(const struct sock *sk, const int flag)
+static inline bool tcp_ack_is_dubious(const struct sock *sk, const int flag)
{
return !(flag & FLAG_NOT_DUP) || (flag & FLAG_CA_ALERT) ||
inet_csk(sk)->icsk_ca_state != TCP_CA_Open;
}
-static inline int tcp_may_raise_cwnd(const struct sock *sk, const int flag)
+static inline bool tcp_may_raise_cwnd(const struct sock *sk, const int flag)
{
const struct tcp_sock *tp = tcp_sk(sk);
return (!(flag & FLAG_ECE) || tp->snd_cwnd < tp->snd_ssthresh) &&
@@ -3387,7 +3387,7 @@ static inline int tcp_may_raise_cwnd(const struct sock *sk, const int flag)
/* Check that window update is acceptable.
* The function assumes that snd_una<=ack<=snd_next.
*/
-static inline int tcp_may_update_window(const struct tcp_sock *tp,
+static inline bool tcp_may_update_window(const struct tcp_sock *tp,
const u32 ack, const u32 ack_seq,
const u32 nwin)
{
@@ -4006,7 +4006,7 @@ static int tcp_disordered_ack(const struct sock *sk, const struct sk_buff *skb)
(s32)(tp->rx_opt.ts_recent - tp->rx_opt.rcv_tsval) <= (inet_csk(sk)->icsk_rto * 1024) / HZ);
}
-static inline int tcp_paws_discard(const struct sock *sk,
+static inline bool tcp_paws_discard(const struct sock *sk,
const struct sk_buff *skb)
{
const struct tcp_sock *tp = tcp_sk(sk);
@@ -4028,7 +4028,7 @@ static inline int tcp_paws_discard(const struct sock *sk,
* (borrowed from freebsd)
*/
-static inline int tcp_sequence(const struct tcp_sock *tp, u32 seq, u32 end_seq)
+static inline bool tcp_sequence(const struct tcp_sock *tp, u32 seq, u32 end_seq)
{
return !before(end_seq, tp->rcv_wup) &&
!after(seq, tp->rcv_nxt + tcp_receive_window(tp));
@@ -5214,7 +5214,7 @@ static __sum16 __tcp_checksum_complete_user(struct sock *sk,
return result;
}
-static inline int tcp_checksum_complete_user(struct sock *sk,
+static inline bool tcp_checksum_complete_user(struct sock *sk,
struct sk_buff *skb)
{
return !skb_csum_unnecessary(skb) &&
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH net-next 4/7] sfc: Add support for IEEE-1588 PTP
From: Richard Cochran @ 2012-07-20 7:37 UTC (permalink / raw)
To: Ben Hutchings
Cc: Stuart Hodgson, David Miller, netdev, linux-net-drivers,
Andrew Jackson
In-Reply-To: <1342713051.2617.40.camel@bwh-desktop.uk.solarflarecom.com>
On Thu, Jul 19, 2012 at 04:50:51PM +0100, Ben Hutchings wrote:
> On Thu, 2012-07-19 at 16:29 +0100, Stuart Hodgson wrote:
> > On 19/07/12 15:25, Richard Cochran wrote:
> [...]
> > > I am trying to purge the whole SYS thing (only blackfin is left)
> > > because there is a much better way to go about this, namely
> > > synchronizing the system time to the PHC time via an internal PPS
> > > signal.
> >
> > This may be possible in future. But leads us to another problem
> > where the PPS event that is generated by the PHC subsystem to the
> > PPS subsystem is stamped with the current system_time. That may
> > be fine for a PPS signal generated from an interrupt but not when
> > the internal PPS event has implicit jitter from the handler/event_queue
> > that we have in the driver.
> [...]
>
> We can certainly take a timestamp in the hard interrupt handler; in fact
> that's what I originally expected we would do since we have a separate
> MSI-X vector for PTP. But even hard interrupt handling can be subject
> to substantial jitter.
What kind of jitter do you see or expect?
I did a study of synching system to PHC on a PowerPC system, where the
PPS timestamps varied from about 10 usec (on average under light load)
to over 30 usec (under heavy load).
Even so, it was easy to synchronize the system clock to within about a
microsecond under light load, with heavy load producing about an
additional 6 usec error.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next] tcp: Return bool instead of int where appropriate
From: Eric Dumazet @ 2012-07-20 8:02 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: netdev, davem
In-Reply-To: <1342769538-4039-1-git-send-email-subramanian.vijay@gmail.com>
On Fri, 2012-07-20 at 00:32 -0700, Vijay Subramanian wrote:
> Applied to a set of static inline functions in tcp_input.c
>
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> ---
> net/ipv4/tcp_input.c | 16 ++++++++--------
> 1 files changed, 8 insertions(+), 8 deletions(-)
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* (unknown)
From: Standard Credit International Finance @ 2012-07-20 8:12 UTC (permalink / raw)
Do you need business loan or personal loan if yes contact us today?
^ permalink raw reply
* [PATCH] ipv4: show pmtu in route list
From: Julian Anastasov @ 2012-07-20 9:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Override the metrics with rt_pmtu
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
Is this patch still useful if routing cache is removed?
net/ipv4/route.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 9f7ffbe..d547f6f 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2909,6 +2909,7 @@ static int rt_fill_info(struct net *net,
struct nlmsghdr *nlh;
unsigned long expires = 0;
u32 error;
+ u32 metrics[RTAX_MAX];
nlh = nlmsg_put(skb, pid, seq, event, sizeof(*r), flags);
if (nlh == NULL)
@@ -2953,7 +2954,10 @@ static int rt_fill_info(struct net *net,
nla_put_be32(skb, RTA_GATEWAY, rt->rt_gateway))
goto nla_put_failure;
- if (rtnetlink_put_metrics(skb, dst_metrics_ptr(&rt->dst)) < 0)
+ memcpy(metrics, dst_metrics_ptr(&rt->dst), sizeof(metrics));
+ if (rt->rt_pmtu)
+ metrics[RTAX_MTU - 1] = rt->rt_pmtu;
+ if (rtnetlink_put_metrics(skb, metrics) < 0)
goto nla_put_failure;
if (rt->rt_mark &&
--
1.7.3.4
^ permalink raw reply related
* [PATCH net-next] tcp: use hash_32() in tcp_metrics
From: Eric Dumazet @ 2012-07-20 9:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
Fix a missing roundup_pow_of_two(), since tcpmhash_entries is not
guaranteed to be a power of two.
Uses hash_32() instead of custom hash.
tcpmhash_entries should be an unsigned int.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/netns/ipv4.h | 2 +-
net/ipv4/tcp_metrics.c | 25 ++++++++++---------------
2 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index d909c7f..0ffb8e3 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -40,7 +40,7 @@ struct netns_ipv4 {
struct sock **icmp_sk;
struct inet_peer_base *peers;
struct tcpm_hash_bucket *tcp_metrics_hash;
- unsigned int tcp_metrics_hash_mask;
+ unsigned int tcp_metrics_hash_log;
struct netns_frags frags;
#ifdef CONFIG_NETFILTER
struct xt_table *iptable_filter;
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 99779ae..992f1bf 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -7,6 +7,7 @@
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/tcp.h>
+#include <linux/hash.h>
#include <net/inet_connection_sock.h>
#include <net/net_namespace.h>
@@ -228,10 +229,8 @@ static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
return NULL;
}
- hash ^= (hash >> 24) ^ (hash >> 16) ^ (hash >> 8);
-
net = dev_net(dst->dev);
- hash &= net->ipv4.tcp_metrics_hash_mask;
+ hash = hash_32(hash, net->ipv4.tcp_metrics_hash_log);
for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
tm = rcu_dereference(tm->tcpm_next)) {
@@ -265,10 +264,8 @@ static struct tcp_metrics_block *__tcp_get_metrics_tw(struct inet_timewait_sock
return NULL;
}
- hash ^= (hash >> 24) ^ (hash >> 16) ^ (hash >> 8);
-
net = twsk_net(tw);
- hash &= net->ipv4.tcp_metrics_hash_mask;
+ hash = hash_32(hash, net->ipv4.tcp_metrics_hash_log);
for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
tm = rcu_dereference(tm->tcpm_next)) {
@@ -302,10 +299,8 @@ static struct tcp_metrics_block *tcp_get_metrics(struct sock *sk,
return NULL;
}
- hash ^= (hash >> 24) ^ (hash >> 16) ^ (hash >> 8);
-
net = dev_net(dst->dev);
- hash &= net->ipv4.tcp_metrics_hash_mask;
+ hash = hash_32(hash, net->ipv4.tcp_metrics_hash_log);
tm = __tcp_get_metrics(&addr, net, hash);
reclaim = false;
@@ -694,7 +689,7 @@ void tcp_fastopen_cache_set(struct sock *sk, u16 mss,
rcu_read_unlock();
}
-static unsigned long tcpmhash_entries;
+static unsigned int tcpmhash_entries;
static int __init set_tcpmhash_entries(char *str)
{
ssize_t ret;
@@ -702,7 +697,7 @@ static int __init set_tcpmhash_entries(char *str)
if (!str)
return 0;
- ret = kstrtoul(str, 0, &tcpmhash_entries);
+ ret = kstrtouint(str, 0, &tcpmhash_entries);
if (ret)
return 0;
@@ -712,7 +707,8 @@ __setup("tcpmhash_entries=", set_tcpmhash_entries);
static int __net_init tcp_net_metrics_init(struct net *net)
{
- int slots, size;
+ size_t size;
+ unsigned int slots;
slots = tcpmhash_entries;
if (!slots) {
@@ -722,14 +718,13 @@ static int __net_init tcp_net_metrics_init(struct net *net)
slots = 8 * 1024;
}
- size = slots * sizeof(struct tcpm_hash_bucket);
+ net->ipv4.tcp_metrics_hash_log = order_base_2(slots);
+ size = sizeof(struct tcpm_hash_bucket) << net->ipv4.tcp_metrics_hash_log;
net->ipv4.tcp_metrics_hash = kzalloc(size, GFP_KERNEL);
if (!net->ipv4.tcp_metrics_hash)
return -ENOMEM;
- net->ipv4.tcp_metrics_hash_mask = (slots - 1);
-
return 0;
}
^ permalink raw reply related
* Re: [PATCH net-next 4/7] sfc: Add support for IEEE-1588 PTP
From: Stuart Hodgson @ 2012-07-20 9:15 UTC (permalink / raw)
To: Richard Cochran
Cc: Ben Hutchings, David Miller, netdev, linux-net-drivers,
Andrew Jackson
In-Reply-To: <20120720063102.GB2330@netboy.at.omicron.at>
>>> This code looks like it is trying to find the offset between two
>>> clocks. Is there some reason why you cannot use <linux/timecompare.h>
>>> to accomplish this?
>>
>> This is what the code is doing. <linux/timecompare.h> states
>>
>> "the assumption is that reading the source
>> time is slow and involves equal time for sending the request and
>> receiving the reply"
>>
>> While in our case event though it is slow we cannot guarantee the second
>> assumption. The code above takes into account some of the particulars of the sfc
>> hardware and gives us good results.
>
> Fair enough, but then maybe a comment mentioning how timecompare is
> unsuitable would be nice to have.
>
This can be added
>>> I am trying to purge the whole SYS thing (only blackfin is left)
>>> because there is a much better way to go about this, namely
>>> synchronizing the system time to the PHC time via an internal PPS
>>> signal.
>>
Do you mean using the PPS kernel consumer to govern the system time?
> I don't understand what the issue is here. Can't you just call
> ptp_clock_event, like you already have...
>
>>>> +static void efx_ptp_pps_worker(struct work_struct *work)
>>>> +{
>>>> + struct efx_ptp_data *ptp =
>>>> + container_of(work, struct efx_ptp_data, pps_work);
>>>> + struct efx_nic *efx = ptp->channel->efx;
>>>> + struct timespec event_gen_time;
>>>> + struct ptp_clock_event ptp_pps_evt;
>>>> + ktime_t gen_time_host;
>>>> +
>>>> + if (efx_ptp_synchronize(efx, PTP_SYNC_ATTEMPTS))
>>>> + return;
>>>> +
>>>> + gen_time_host = ktime_sub(ptp->mc_base_time,
>>>> + ptp->host_base_time);
>>>> + event_gen_time = ktime_to_timespec(gen_time_host);
>>>> +
>>>> + ptp_pps_evt.type = PTP_CLOCK_EXTTS;
>>>> + ptp_pps_evt.timestamp = ktime_to_ns(gen_time_host);
>>>> + ptp_clock_event(ptp->phc_clock, &ptp_pps_evt);
>>>> +}
>
> ... here?
>
In order for a PPS to arrive at the kernel consumer ptp_clock_event
needs to be called with PTP_CLOCK_PPS. This then calls pps_get_ts
and stamps the event with the current system time, not the time
that was put into the event.
Using PTP_CLOCK_EXTTS the PPS is visible to userspace via a read
on the phc device and can then be used in our modified ptpd2.
>>>> +static int efx_phc_adjtime(struct ptp_clock_info *ptp, s64 delta)
>>>> +{
>
> You can set the time here somehow by doing, T' = T + offset, and so...
>
>>>> +}
>
>>>> +static int efx_phc_settime(struct ptp_clock_info *ptp,
>>>> + const struct timespec *e_ts)
>>>> +{
>>>> + /* We must provide this function, but we cannot actually set the time */
>>>
>>> Huh? You can adjtime, so must be able to settime, too, right?
>>>
>>> If you have enough range in the RAW timestamp in the MC firmware (like
>>> 64 bits of nanoseconds), and you allow settime, then you can spare the
>>> system time synchronization code altogether.
>>>
>>
>> You will have to elaborate further on this point.
>
> ... why can't you also just set the time?
Our hardware can only have an offset applied to the clock. In order to set time
we need to know the time now, then work out and offset to get to the target time.
At the point that we apply this offset the clock will have moved on and not be
set to the target time. We can apply some measured average times to the offset
to get closer but with this hardware settime will not leave the NIC clock at the
desired time.
>
> Thanks,
> Richard
^ permalink raw reply
* Re: [PATCH] net, cgroup: Fix boot failure due to iteration of uninitialized list
From: Srivatsa S. Bhat @ 2012-07-20 10:04 UTC (permalink / raw)
To: Neil Horman
Cc: gaofeng, eric.dumazet, davem, linux-kernel, netdev, mark.d.rustad,
john.r.fastabend, lizefan
In-Reply-To: <20120719164407.GA2963@neilslaptop.think-freely.org>
On 07/19/2012 10:14 PM, Neil Horman wrote:
> On Thu, Jul 19, 2012 at 09:57:37PM +0530, Srivatsa S. Bhat wrote:
>> After commit ef209f15 (net: cgroup: fix access the unallocated memory in
>> netprio cgroup), boot fails with the following NULL pointer dereference:
>>
>> Initializing cgroup subsys devices
>> Initializing cgroup subsys freezer
>> Initializing cgroup subsys net_cls
>> Initializing cgroup subsys blkio
>> Initializing cgroup subsys perf_event
>> Initializing cgroup subsys net_prio
>> BUG: unable to handle kernel NULL pointer dereference at 0000000000000698
>> IP: [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
>> PGD 0
>> Oops: 0000 [#1] SMP
>> CPU 0
>> Modules linked in:
>>
>> Pid: 0, comm: swapper/0 Not tainted 3.5.0-rc7-mandeep #1 IBM IBM System x -[7870C4Q]-/68Y8033
>> RIP: 0010:[<ffffffff8145e8d6>] [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
>> RSP: 0000:ffffffff81a01ea8 EFLAGS: 00010213
>> RAX: 0000000000000000 RBX: ffffffffffffff10 RCX: 0000000000000000
>> RDX: 0000000000000000 RSI: 0000000000000246 RDI: ffffffff81aa70a0
>> RBP: ffffffff81a01ed8 R08: 0000000000000000 R09: 0000000000000000
>> R10: ffff8808ff8641c0 R11: 6e697a696c616974 R12: 0000000000000001
>> R13: ffff8808ff8641c0 R14: 0000000000000000 R15: 0000000000093970
>> FS: 0000000000000000(0000) GS:ffff8808ffc00000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>> CR2: 0000000000000698 CR3: 0000000001a0b000 CR4: 00000000000006b0
>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
>> Process swapper/0 (pid: 0, threadinfo ffffffff81a00000, task ffffffff81a13420)
>> Stack:
>> ffffffff81a01eb8 ffffffff818060ff ffffffff81d75ec8 ffffffff81aa8960
>> ffffffff81aa8960 ffffffff81b4c2c0 ffffffff81a01ef8 ffffffff81b1cb78
>> 0000000000000018 0000000000000048 ffffffff81a01f18 ffffffff81b1ce13
>> Call Trace:
>> [<ffffffff81b1cb78>] cgroup_init_subsys+0x83/0x169
>> [<ffffffff81b1ce13>] cgroup_init+0x36/0x119
>> [<ffffffff81affef7>] start_kernel+0x3ba/0x3ef
>> [<ffffffff81aff95b>] ? kernel_init+0x27b/0x27b
>> [<ffffffff81aff356>] x86_64_start_reservations+0x131/0x136
>> [<ffffffff81aff45e>] x86_64_start_kernel+0x103/0x112
>> Code: 01 48 3d f8 e1 ec 81 48 8d 98 10 ff ff ff 75 1b eb 73 0f 1f 00 48 8b 83 f0 00 00 00 48 3d f8 e1 ec 81 48 8d 98 10 ff ff ff 74 5a <48> 8b 83 88 07 00 00 48 85 c0 74 de 44 3b 60 10 76 d8 44 89 e6
>> RIP [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
>> RSP <ffffffff81a01ea8>
>> CR2: 0000000000000698
>> ---[ end trace a7919e7f17c0a725 ]---
>> Kernel panic - not syncing: Attempted to kill the idle task!
>>
>> The code corresponds to:
>>
>> update_netdev_tables():
>> for_each_netdev(&init_net, dev) {
>> map = rtnl_dereference(dev->priomap); <---- HERE
>>
>>
>> The list head is initialized in netdev_init(), which is called much
>> later than cgrp_create(). So the problem is that we are calling
>> update_netdev_tables() way too early (in cgrp_create()), which will
>> end up traversing the not-yet-circular linked list. So at some point,
>> the dev pointer will become NULL and hence dev->priomap becomes an
>> invalid access.
>>
>> To fix this, just remove the update_netdev_tables() function entirely,
>> since it appears that write_update_netdev_table() will handle things
>> just fine.
>>
>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
>> ---
>>
>> Requesting a thorough review of this patch, since I am not sure whether
>> removing update_netdev_tables() is perfectly OK and whether that is the
>> right thing to do.
>>
> We could do this I suppose, but this has already been fixed by
> 734b65417b24d6eea3e3d7457e1f11493890ee1d
Oh good! But don't you think that my patch looks cleaner than that fix?
(Of course, provided that my patch is correct!)
Anyway, I'm happy to see that the boot failure is fixed. But if anyone feels
that the approach of removing the update_netdev_tables() function is correct
and better, I'll be happy to provide a patch on top of the boot-fix that
went upstream.
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [RFC] r8169 : why SG / TX checksum are default disabled
From: Francois Romieu @ 2012-07-20 10:08 UTC (permalink / raw)
To: hayeswang; +Cc: 'David Miller', eric.dumazet, netdev
In-Reply-To: <EF06A7B3C1E6432BB660CDEF8E1D2A1B@realtek.com.tw>
hayeswang <hayeswang@realtek.com> :
[...]
> I find that the total length field of IP header would be modified if the hw
> checksum is enabled. Therefore, skb_padto + hw checksum wouldn't work.
Ok, my patch completely ignored the fact that skb_padto does not change the
length.
However skb_padto + length adjustement + hw checksum should work (at least in
theory if not in the patch below) ?
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index be4e00f..8d0cc09 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -5740,7 +5740,7 @@ err_out:
return -EIO;
}
-static inline void rtl8169_tso_csum(struct rtl8169_private *tp,
+static inline bool rtl8169_tso_csum(struct rtl8169_private *tp,
struct sk_buff *skb, u32 *opts)
{
const struct rtl_tx_desc_info *info = tx_desc_info + tp->txd_version;
@@ -5753,6 +5753,13 @@ static inline void rtl8169_tso_csum(struct rtl8169_private *tp,
} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
const struct iphdr *ip = ip_hdr(skb);
+ if (unlikely(skb->len < ETH_ZLEN &&
+ (tp->mac_version == RTL_GIGA_MAC_VER_34))) {
+ if (skb_padto(skb, ETH_ZLEN))
+ return false;
+ skb_put(skb, ETH_ZLEN - skb->len);
+ }
+
if (ip->protocol == IPPROTO_TCP)
opts[offset] |= info->checksum.tcp;
else if (ip->protocol == IPPROTO_UDP)
@@ -5760,6 +5767,7 @@ static inline void rtl8169_tso_csum(struct rtl8169_private *tp,
else
WARN_ON_ONCE(1);
}
+ return true;
}
static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
@@ -5783,25 +5791,26 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
if (unlikely(le32_to_cpu(txd->opts1) & DescOwn))
goto err_stop_0;
+ opts[1] = cpu_to_le32(rtl8169_tx_vlan_tag(tp, skb));
+ opts[0] = DescOwn;
+
+ if (!rtl8169_tso_csum(tp, skb, opts))
+ goto err_update_stats_0;
+
len = skb_headlen(skb);
mapping = dma_map_single(d, skb->data, len, DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(d, mapping))) {
if (net_ratelimit())
netif_err(tp, drv, dev, "Failed to map TX DMA!\n");
- goto err_dma_0;
+ goto err_free_skb_1;
}
tp->tx_skb[entry].len = len;
txd->addr = cpu_to_le64(mapping);
- opts[1] = cpu_to_le32(rtl8169_tx_vlan_tag(tp, skb));
- opts[0] = DescOwn;
-
- rtl8169_tso_csum(tp, skb, opts);
-
frags = rtl8169_xmit_frags(tp, skb, opts);
if (frags < 0)
- goto err_dma_1;
+ goto err_unmap_2;
else if (frags)
opts[0] |= FirstFrag;
else {
@@ -5849,10 +5858,11 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
return NETDEV_TX_OK;
-err_dma_1:
+err_unmap_2:
rtl8169_unmap_tx_skb(d, tp->tx_skb + entry, txd);
-err_dma_0:
+err_free_skb_1:
dev_kfree_skb(skb);
+err_update_stats_0:
dev->stats.tx_dropped++;
return NETDEV_TX_OK;
^ permalink raw reply related
* RE: [PATCH V2 resend] ipv6: fix incorrect route 'expires' value passed to userspace
From: David Laight @ 2012-07-20 10:32 UTC (permalink / raw)
To: Li Wei, David Miller; +Cc: netdev, shemminger
In-Reply-To: <5008B794.7010904@cn.fujitsu.com>
> - else if (rt->dst.expires - jiffies < INT_MAX)
> - expires = rt->dst.expires - jiffies;
> + else if ((long)rt->dst.expires - (long)jiffies > INT_MIN
> + && (long)rt->dst.expires - (long)jiffies <
INT_MAX)
> + expires = (long)rt->dst.expires - (long)jiffies;
> else
> - expires = INT_MAX;
> + expires = time_is_after_jiffies(rt->dst.expires) ?
INT_MAX : INT_MIN;
I can't help feeling there is a better way to do this.
Maybe:
long expires = rt->dst.expires - jiffies;
if (expires != (int)expires)
expires = expires > 0 ? INT_MAX : INT_MIN;
Although maybe -INT_MAX instead of INT_MIN.
David
^ permalink raw reply
* Re: [PATCH] net, cgroup: Fix boot failure due to iteration of uninitialized list
From: Neil Horman @ 2012-07-20 11:00 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: gaofeng, eric.dumazet, davem, linux-kernel, netdev, mark.d.rustad,
john.r.fastabend, lizefan
In-Reply-To: <50092D3F.5020108@linux.vnet.ibm.com>
On Fri, Jul 20, 2012 at 03:34:47PM +0530, Srivatsa S. Bhat wrote:
> On 07/19/2012 10:14 PM, Neil Horman wrote:
> > On Thu, Jul 19, 2012 at 09:57:37PM +0530, Srivatsa S. Bhat wrote:
> >> After commit ef209f15 (net: cgroup: fix access the unallocated memory in
> >> netprio cgroup), boot fails with the following NULL pointer dereference:
> >>
> >> Initializing cgroup subsys devices
> >> Initializing cgroup subsys freezer
> >> Initializing cgroup subsys net_cls
> >> Initializing cgroup subsys blkio
> >> Initializing cgroup subsys perf_event
> >> Initializing cgroup subsys net_prio
> >> BUG: unable to handle kernel NULL pointer dereference at 0000000000000698
> >> IP: [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
> >> PGD 0
> >> Oops: 0000 [#1] SMP
> >> CPU 0
> >> Modules linked in:
> >>
> >> Pid: 0, comm: swapper/0 Not tainted 3.5.0-rc7-mandeep #1 IBM IBM System x -[7870C4Q]-/68Y8033
> >> RIP: 0010:[<ffffffff8145e8d6>] [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
> >> RSP: 0000:ffffffff81a01ea8 EFLAGS: 00010213
> >> RAX: 0000000000000000 RBX: ffffffffffffff10 RCX: 0000000000000000
> >> RDX: 0000000000000000 RSI: 0000000000000246 RDI: ffffffff81aa70a0
> >> RBP: ffffffff81a01ed8 R08: 0000000000000000 R09: 0000000000000000
> >> R10: ffff8808ff8641c0 R11: 6e697a696c616974 R12: 0000000000000001
> >> R13: ffff8808ff8641c0 R14: 0000000000000000 R15: 0000000000093970
> >> FS: 0000000000000000(0000) GS:ffff8808ffc00000(0000) knlGS:0000000000000000
> >> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> >> CR2: 0000000000000698 CR3: 0000000001a0b000 CR4: 00000000000006b0
> >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> >> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> >> Process swapper/0 (pid: 0, threadinfo ffffffff81a00000, task ffffffff81a13420)
> >> Stack:
> >> ffffffff81a01eb8 ffffffff818060ff ffffffff81d75ec8 ffffffff81aa8960
> >> ffffffff81aa8960 ffffffff81b4c2c0 ffffffff81a01ef8 ffffffff81b1cb78
> >> 0000000000000018 0000000000000048 ffffffff81a01f18 ffffffff81b1ce13
> >> Call Trace:
> >> [<ffffffff81b1cb78>] cgroup_init_subsys+0x83/0x169
> >> [<ffffffff81b1ce13>] cgroup_init+0x36/0x119
> >> [<ffffffff81affef7>] start_kernel+0x3ba/0x3ef
> >> [<ffffffff81aff95b>] ? kernel_init+0x27b/0x27b
> >> [<ffffffff81aff356>] x86_64_start_reservations+0x131/0x136
> >> [<ffffffff81aff45e>] x86_64_start_kernel+0x103/0x112
> >> Code: 01 48 3d f8 e1 ec 81 48 8d 98 10 ff ff ff 75 1b eb 73 0f 1f 00 48 8b 83 f0 00 00 00 48 3d f8 e1 ec 81 48 8d 98 10 ff ff ff 74 5a <48> 8b 83 88 07 00 00 48 85 c0 74 de 44 3b 60 10 76 d8 44 89 e6
> >> RIP [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
> >> RSP <ffffffff81a01ea8>
> >> CR2: 0000000000000698
> >> ---[ end trace a7919e7f17c0a725 ]---
> >> Kernel panic - not syncing: Attempted to kill the idle task!
> >>
> >> The code corresponds to:
> >>
> >> update_netdev_tables():
> >> for_each_netdev(&init_net, dev) {
> >> map = rtnl_dereference(dev->priomap); <---- HERE
> >>
> >>
> >> The list head is initialized in netdev_init(), which is called much
> >> later than cgrp_create(). So the problem is that we are calling
> >> update_netdev_tables() way too early (in cgrp_create()), which will
> >> end up traversing the not-yet-circular linked list. So at some point,
> >> the dev pointer will become NULL and hence dev->priomap becomes an
> >> invalid access.
> >>
> >> To fix this, just remove the update_netdev_tables() function entirely,
> >> since it appears that write_update_netdev_table() will handle things
> >> just fine.
> >>
> >> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
> >> ---
> >>
> >> Requesting a thorough review of this patch, since I am not sure whether
> >> removing update_netdev_tables() is perfectly OK and whether that is the
> >> right thing to do.
> >>
> > We could do this I suppose, but this has already been fixed by
> > 734b65417b24d6eea3e3d7457e1f11493890ee1d
>
> Oh good! But don't you think that my patch looks cleaner than that fix?
> (Of course, provided that my patch is correct!)
>
> Anyway, I'm happy to see that the boot failure is fixed. But if anyone feels
> that the approach of removing the update_netdev_tables() function is correct
> and better, I'll be happy to provide a patch on top of the boot-fix that
> went upstream.
>
We're almost at the end of a release. The fix that went in has been tesetd and
fixes the specific problem that was reported, with almost zero likelyhood of
causing other regressions. While this fix looks like it might be preferable,
this isn't a time to go doing something like this without alot more testing, as
it may cause unforseen problems.
Theres also a larger issue of initalization order that I'll be looking at in the
next few weeks. Based on the outcome of that I may roll this change in.
Thanks!
Neil
> Regards,
> Srivatsa S. Bhat
>
>
^ permalink raw reply
* Re: [PATCH] net, cgroup: Fix boot failure due to iteration of uninitialized list
From: Srivatsa S. Bhat @ 2012-07-20 11:18 UTC (permalink / raw)
To: Neil Horman
Cc: gaofeng, eric.dumazet, davem, linux-kernel, netdev, mark.d.rustad,
john.r.fastabend, lizefan
In-Reply-To: <20120720110016.GA22367@hmsreliant.think-freely.org>
On 07/20/2012 04:30 PM, Neil Horman wrote:
> On Fri, Jul 20, 2012 at 03:34:47PM +0530, Srivatsa S. Bhat wrote:
>> On 07/19/2012 10:14 PM, Neil Horman wrote:
>>> On Thu, Jul 19, 2012 at 09:57:37PM +0530, Srivatsa S. Bhat wrote:
>>>> After commit ef209f15 (net: cgroup: fix access the unallocated memory in
>>>> netprio cgroup), boot fails with the following NULL pointer dereference:
>>>>
>>>> Initializing cgroup subsys devices
>>>> Initializing cgroup subsys freezer
>>>> Initializing cgroup subsys net_cls
>>>> Initializing cgroup subsys blkio
>>>> Initializing cgroup subsys perf_event
>>>> Initializing cgroup subsys net_prio
>>>> BUG: unable to handle kernel NULL pointer dereference at 0000000000000698
>>>> IP: [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
>>>> PGD 0
>>>> Oops: 0000 [#1] SMP
>>>> CPU 0
>>>> Modules linked in:
>>>>
>>>> Pid: 0, comm: swapper/0 Not tainted 3.5.0-rc7-mandeep #1 IBM IBM System x -[7870C4Q]-/68Y8033
>>>> RIP: 0010:[<ffffffff8145e8d6>] [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
>>>> RSP: 0000:ffffffff81a01ea8 EFLAGS: 00010213
>>>> RAX: 0000000000000000 RBX: ffffffffffffff10 RCX: 0000000000000000
>>>> RDX: 0000000000000000 RSI: 0000000000000246 RDI: ffffffff81aa70a0
>>>> RBP: ffffffff81a01ed8 R08: 0000000000000000 R09: 0000000000000000
>>>> R10: ffff8808ff8641c0 R11: 6e697a696c616974 R12: 0000000000000001
>>>> R13: ffff8808ff8641c0 R14: 0000000000000000 R15: 0000000000093970
>>>> FS: 0000000000000000(0000) GS:ffff8808ffc00000(0000) knlGS:0000000000000000
>>>> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>>>> CR2: 0000000000000698 CR3: 0000000001a0b000 CR4: 00000000000006b0
>>>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>>> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
>>>> Process swapper/0 (pid: 0, threadinfo ffffffff81a00000, task ffffffff81a13420)
>>>> Stack:
>>>> ffffffff81a01eb8 ffffffff818060ff ffffffff81d75ec8 ffffffff81aa8960
>>>> ffffffff81aa8960 ffffffff81b4c2c0 ffffffff81a01ef8 ffffffff81b1cb78
>>>> 0000000000000018 0000000000000048 ffffffff81a01f18 ffffffff81b1ce13
>>>> Call Trace:
>>>> [<ffffffff81b1cb78>] cgroup_init_subsys+0x83/0x169
>>>> [<ffffffff81b1ce13>] cgroup_init+0x36/0x119
>>>> [<ffffffff81affef7>] start_kernel+0x3ba/0x3ef
>>>> [<ffffffff81aff95b>] ? kernel_init+0x27b/0x27b
>>>> [<ffffffff81aff356>] x86_64_start_reservations+0x131/0x136
>>>> [<ffffffff81aff45e>] x86_64_start_kernel+0x103/0x112
>>>> Code: 01 48 3d f8 e1 ec 81 48 8d 98 10 ff ff ff 75 1b eb 73 0f 1f 00 48 8b 83 f0 00 00 00 48 3d f8 e1 ec 81 48 8d 98 10 ff ff ff 74 5a <48> 8b 83 88 07 00 00 48 85 c0 74 de 44 3b 60 10 76 d8 44 89 e6
>>>> RIP [<ffffffff8145e8d6>] cgrp_create+0xf6/0x190
>>>> RSP <ffffffff81a01ea8>
>>>> CR2: 0000000000000698
>>>> ---[ end trace a7919e7f17c0a725 ]---
>>>> Kernel panic - not syncing: Attempted to kill the idle task!
>>>>
>>>> The code corresponds to:
>>>>
>>>> update_netdev_tables():
>>>> for_each_netdev(&init_net, dev) {
>>>> map = rtnl_dereference(dev->priomap); <---- HERE
>>>>
>>>>
>>>> The list head is initialized in netdev_init(), which is called much
>>>> later than cgrp_create(). So the problem is that we are calling
>>>> update_netdev_tables() way too early (in cgrp_create()), which will
>>>> end up traversing the not-yet-circular linked list. So at some point,
>>>> the dev pointer will become NULL and hence dev->priomap becomes an
>>>> invalid access.
>>>>
>>>> To fix this, just remove the update_netdev_tables() function entirely,
>>>> since it appears that write_update_netdev_table() will handle things
>>>> just fine.
>>>>
>>>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
>>>> ---
>>>>
>>>> Requesting a thorough review of this patch, since I am not sure whether
>>>> removing update_netdev_tables() is perfectly OK and whether that is the
>>>> right thing to do.
>>>>
>>> We could do this I suppose, but this has already been fixed by
>>> 734b65417b24d6eea3e3d7457e1f11493890ee1d
>>
>> Oh good! But don't you think that my patch looks cleaner than that fix?
>> (Of course, provided that my patch is correct!)
>>
>> Anyway, I'm happy to see that the boot failure is fixed. But if anyone feels
>> that the approach of removing the update_netdev_tables() function is correct
>> and better, I'll be happy to provide a patch on top of the boot-fix that
>> went upstream.
>>
> We're almost at the end of a release. The fix that went in has been tesetd and
> fixes the specific problem that was reported, with almost zero likelyhood of
> causing other regressions. While this fix looks like it might be preferable,
> this isn't a time to go doing something like this without alot more testing, as
> it may cause unforseen problems.
>
Oh definitely! I didn't mean to suggest doing these changes right away.
It can surely wait.. :)
> Theres also a larger issue of initalization order that I'll be looking at in the
> next few weeks. Based on the outcome of that I may roll this change in.
>
Sure, thanks!
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* WESTERN UNION COMPENSATION PAYMENT
From: South Africa western union @ 2012-07-20 11:58 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2: WUMT.docx --]
[-- Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document, Size: 27936 bytes --]
^ permalink raw reply
* [patch net-next 0/6] net: add team multiqueue support and do comple of thing on the way
From: Jiri Pirko @ 2012-07-20 12:28 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, shemminger, fubar, andy
This patchset represents the way I walked when I was adding multiqueue support for
team driver.
Jiri Pirko (6):
net: honour netif_set_real_num_tx_queues() retval
rtnl: allow to specify different num for rx and tx queue count
rtnl: allow to specify number of rx and tx queues on device creation
net: rename bond_queue_mapping to slave_dev_queue_mapping
bond_sysfs: use ream_num_tx_queues rather than params.tx_queue
team: add multiqueue support
drivers/net/bonding/bond_main.c | 20 ++++++------
drivers/net/bonding/bond_sysfs.c | 2 +-
drivers/net/team/team.c | 65 +++++++++++++++++++++++++++++++++++---
include/linux/if_link.h | 2 ++
include/linux/if_team.h | 8 +++++
include/linux/netdevice.h | 7 +++-
include/net/rtnetlink.h | 10 +++---
include/net/sch_generic.h | 2 +-
net/core/rtnetlink.c | 27 +++++++++++-----
9 files changed, 114 insertions(+), 29 deletions(-)
--
1.7.10.4
^ 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