* Re: iproute2 - IPsec ESN support
From: Steffen Klassert @ 2012-07-25 9:51 UTC (permalink / raw)
To: Geanta Neag Horia Ioan-B05471
Cc: netdev@vger.kernel.org, linux-crypto@vger.kernel.org,
Stephen Hemminger
In-Reply-To: <FD18E5D573A8AB48A365D4D78185DE992602BD@039-SN1MPN1-001.039d.mgd.msft.net>
On Wed, Jul 25, 2012 at 06:14:52AM +0000, Geanta Neag Horia Ioan-B05471 wrote:
> Hello,
>
> Is there any way to create an IPsec tunnel and indicate using
> extended sequnce numbers?
The strongswan ike deamon supports extended sequnce numbers.
>
> It seems that currently iproute2 doesn't support this.
> Grepping for "esn" reveals that XFRM_STATE_ESN shows only in kernel headers.
>
> The only relevant thing I found was a RFC sent by Steffen (Cc-ed),
> but it was never applied (don't know why):
> [RFC] iproute2: Add IPsec extended sequence number support
I'll take this as a reminder to respin this patch.
^ permalink raw reply
* skb_ts_get_next_block panic
From: kendo @ 2012-07-25 9:52 UTC (permalink / raw)
To: netdev
My server every few days, or more than 10 days will restart,Probably because of the use of the Netfilter xt_string module,
But it seems not xt_string bug, but because of the bogus SKB buffer:
Jul 25 15:13:18 AnShion <9> klogd: [ 5413.055010] BUG: unable to handle kernel paging request at 2f3c0a3e
Jul 25 15:13:18 AnShion <9> klogd: [ 5413.130063] IP: [<c012c928>] kmap_atomic_prot+0x18/0xf0
Jul 25 15:13:18 AnShion <12> klogd: [ 5413.192555] *pde = 00000000
Jul 25 15:13:18 AnShion <8> klogd: [ 5413.227015] Oops: 0000 [#1] SMP
Jul 25 15:13:18 AnShion <8> klogd: [ 5413.265738] last sysfs file: /sys/devices/virtual/net/bond0/bonding/slaves
Jul 25 15:13:18 AnShion <12> klogd: [ 5413.347824] Modules linked in: igb e1000e
Jul 25 15:13:18 AnShion <12> klogd: [ 5413.578307]
Jul 25 15:13:18 AnShion <12> klogd: [ 5413.596059] Pid: 0, comm: kworker/0:0 Tainted: G M 2.6.38.8 #412 Intel Corporation S1200BTL/S1200BTL
Jul 25 15:13:18 AnShion <12> klogd: [ 5413.715011] EIP: 0060:[<c012c928>] EFLAGS: 00010202 CPU: 1
Jul 25 15:13:18 AnShion <12> klogd: [ 5413.780491] EIP is at kmap_atomic_prot+0x18/0xf0
Jul 25 15:13:18 AnShion <12> klogd: [ 5413.835595] EAX: f48b2000 EBX: 00000163 ECX: 2f3c0a3e EDX: 00000163
Jul 25 15:13:18 AnShion <12> klogd: [ 5413.910417] ESI: f1f95e40 EDI: 00000000 EBP: f48b3bc4 ESP: f48b3bb4
Jul 25 15:13:18 AnShion <12> klogd: [ 5413.985237] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
Jul 25 15:13:18 AnShion <8> klogd: [ 5414.049680] Process kworker/0:0 (pid: 0, ti=f48b2000 task=f48a8000 task.ti=f489e000)
Jul 25 15:13:18 AnShion <8> klogd: [ 5414.142138] Stack:
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.166117] 00000001 f48b3c6c f1f95e40 00000000 f48b3bcc c012ca0e f48b3bf4 c069ecde
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.259457] 00000004 eb636a80 f48b3c28 000005f8 00007365 00000000 f48b3c68 f1f9584e
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.352797] f48b3bfc c069ed1e f48b3c38 c038c2da f48b3c68 f48b3c68 f11c9d20 f11c9d30
Jul 25 15:13:18 AnShion <8> klogd: [ 5414.446140] Call Trace:
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.475306] [<c012ca0e>] __kmap_atomic+0xe/0x10
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.530416] [<c069ecde>] skb_seq_read+0x19e/0x1d0
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.587597] [<c069ed1e>] skb_ts_get_next_block+0xe/0x10
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.651007] [<c038c2da>] kmp_find+0x3a/0x150
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.703003] [<c069c842>] skb_find_text+0x62/0x90
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.759145] [<c06faa3b>] string_mt+0x5b/0x90
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.811140] [<c074a6b5>] ipt_do_table+0x295/0x3b0
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.868323] [<c074b020>] iptable_mangle_hook+0x40/0x150
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.931729] [<c06db8e7>] nf_iterate+0x67/0x90
Jul 25 15:13:18 AnShion <12> klogd: [ 5414.984760] [<c0702040>] ? ip_forward_finish+0x0/0x60
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.046089] [<c06dba90>] nf_hook_slow+0xa0/0xf0
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.101195] [<c0702040>] ? ip_forward_finish+0x0/0x60
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.162525] [<c070227d>] ip_forward+0x1dd/0x430
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.217631] [<c0702040>] ? ip_forward_finish+0x0/0x60
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.278962] [<c0700941>] ip_rcv_finish+0x241/0x3c0
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.337181] [<c0700d66>] ip_rcv+0x2a6/0x320
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.388142] [<c0700700>] ? ip_rcv_finish+0x0/0x3c0
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.446365] [<c06a9518>] __netif_receive_skb+0x258/0x520
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.510811] [<c01e8d10>] ? add_partial+0x40/0x70
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.566956] [<c06a9913>] netif_receive_skb+0x23/0x50
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.627248] [<c06a9a37>] napi_skb_finish+0x37/0x50
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.685469] [<c06aa07b>] napi_gro_receive+0xdb/0xf0
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.744722] [<c069d8bf>] ? consume_skb+0x4f/0x70
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.800881] [<f815c79c>] igb_poll+0x5fc/0xef0 [igb]
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.860137] [<c0819538>] ? _raw_spin_lock+0x8/0x10
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.918359] [<c01e8d10>] ? add_partial+0x40/0x70
Jul 25 15:13:18 AnShion <12> klogd: [ 5415.974502] [<c01eb8e7>] ? __slab_free+0xc7/0xd0
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.030645] [<c01eb8e7>] ? __slab_free+0xc7/0xd0
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.086787] [<c0819538>] ? _raw_spin_lock+0x8/0x10
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.145008] [<c0125708>] ? default_spin_lock_flags+0x8/0x10
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.212568] [<c0819568>] ? _raw_spin_lock_irqsave+0x28/0x40
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.280124] [<c06a9eaa>] net_rx_action+0xaa/0x1a0
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.337306] [<c014bff1>] __do_softirq+0xb1/0x190
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.393445] [<c014bf40>] ? __do_softirq+0x0/0x190
Jul 25 15:13:18 AnShion <8> klogd: [ 5416.450626] <IRQ>
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.475750] [<c014bebd>] ? irq_exit+0x5d/0x80
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.528782] [<c0104bc5>] ? do_IRQ+0x45/0xb0
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.579736] [<c014bed5>] ? irq_exit+0x75/0x80
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.632767] [<c011c9f6>] ? smp_apic_timer_interrupt+0x56/0x90
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.702403] [<c01036f0>] ? common_interrupt+0x30/0x38
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.763733] [<c040e42e>] ? acpi_idle_enter_c1+0x78/0x95
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.827142] [<c06740d9>] ? cpuidle_idle_call+0xd9/0x1c0
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.890545] [<c010214a>] ? cpu_idle+0x8a/0xc0
Jul 25 15:13:18 AnShion <12> klogd: [ 5416.943574] [<c0813979>] ? start_secondary+0x1a1/0x1e8
Jul 25 15:13:18 AnShion <8> klogd: [ 5417.005943] Code: 74 26 00 8b 15 8c a4 d5 c1 55 89 e5 e8 e2 f8 ff ff 5d c3 55 89 c1 89 e5 57 56 53 89 d3 83 ec 04 89 e0 25 00 e0 ff ff 83 40 14 01 <8b> 01 c1 e8 1e 69 c0 80 03 00 00 05 00 79 b3 c0 2b 80 4c 03 00
^ permalink raw reply
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Neil Horman @ 2012-07-25 11:15 UTC (permalink / raw)
To: Xufeng Zhang
Cc: xufeng zhang, vyasevich, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CA+=dFzjW_=T6DwtRrSCxjzE9HKKvpQ=LUmGpAggJEujZT+VX+g@mail.gmail.com>
On Wed, Jul 25, 2012 at 10:34:32AM +0800, Xufeng Zhang wrote:
> On 7/24/12, Neil Horman <nhorman@tuxdriver.com> wrote:
> > On Tue, Jul 24, 2012 at 09:50:18AM +0800, xufeng zhang wrote:
> >> On 07/23/2012 08:14 PM, Neil Horman wrote:
> >> >On Mon, Jul 23, 2012 at 10:30:34AM +0800, xufeng zhang wrote:
> >> >>On 07/23/2012 08:49 AM, Neil Horman wrote:
> >> >>>Not sure I understand how you came into this error. If we get an
> >> >>> invalid
> >> >>>stream, we issue an SCTP_REPORT_TSN side effect, followed by an
> >> >>> SCTP_CMD_REPLY
> >> >>>which sends the error chunk. The reply goes through
> >> >>>sctp_outq_tail->sctp_outq_chunk->sctp_outq_transmit_chunk->sctp_outq_append_chunk.
> >> >>>That last function checks to see if a sack is already part of the
> >> >>> packet, and if
> >> >>>there isn't one, appends one, using the updated tsn map.
> >> >>Yes, you are right, but consider the invalid stream identifier's
> >> >>DATA chunk is the first
> >> >>DATA chunk in the association which will need SACK immediately.
> >> >>Here is what I thought of the scenario:
> >> >> sctp_sf_eat_data_6_2()
> >> >> -->sctp_eat_data()
> >> >> -->sctp_make_op_error()
> >> >> -->sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> >> >> SCTP_CHUNK(err))
> >> >> -->sctp_outq_tail() /* First enqueue ERROR chunk
> >> >> */
> >> >> -->sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, SCTP_FORCE())
> >> >> -->sctp_gen_sack()
> >> >> -->sctp_make_sack()
> >> >> -->sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> >> >>SCTP_CHUNK(sack))
> >> >> -->sctp_outq_tail() /* Then enqueue SACK chunk
> >> >> */
> >> >>
> >> >>So SACK chunk is enqueued after ERROR chunk.
> >> >Ah, I see. Since the ERROR and SACK chunks are both control chunks, and
> >> > since
> >> >we explicitly add the SACK to the control queue instead of going through
> >> > the
> >> >bundle path in sctp_packet_append_chunk the ordering gets wrong.
> >> >
> >> >Ok, so the problem makes sense. I think the soultion could be alot
> >> > easier
> >> >though. IIRC SACK chunks always live at the head of a packet, so why not
> >> > just
> >> >special case it in sctp_outq_tail? I.e. instead of doing a
> >> > list_add_tail, in
> >> >the else clause of sctp_outq_tail check the chunk_hdr->type to see if
> >> > its
> >> >SCTP_CID_SACK. If it is, use list_add_head rather than list_add_tail. I
> >> > think
> >> >that will fix up both the COOKIE_ECHO and ESTABLISHED cases, won't it?
> >> > And then
> >> >you won't have keep track of extra state in the packet configuration.
> >> Yes, it's a good idea, but I think the premise is not correct:
> >> RFC 4960 page 57:
> >> "D) Upon reception of the COOKIE ECHO chunk, endpoint "Z" will reply
> >> with a COOKIE ACK chunk after building a TCB and moving to the
> >> ESTABLISHED state. A COOKIE ACK chunk may be bundled with any
> >> pending DATA chunks (and/or SACK chunks), *but the COOKIE ACK chunk
> >> MUST be the first chunk in the packet*."
> >>
> >> So we can't put SACK chunk always at the head of the packet.
> >>
> > Ok, Fair point, but that just changes the ordering a bit to:
> > COOKIE_ACK
> > SACK
> > OTHER CONTROL CHUNKS
> >
> > What about something like this? Its completely untested, and I'm sure it
> > can be
> > cleaned up a bunch, but this keeps us from having to add additional state to
> > the
> > packet structure.
> Yeah! I like this modification, thank you very much for your work!
> I'll try to send a V2 patch based on your changes and run some tests.
>
Awesome, thank you!
Neil
^ permalink raw reply
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Neil Horman @ 2012-07-25 11:27 UTC (permalink / raw)
To: Xufeng Zhang
Cc: Vlad Yasevich, xufeng zhang, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CA+=dFzgXq3Sx-Ny+6oDZ3BEdsMQCgswNEVPVnhmhedHFvazzWw@mail.gmail.com>
On Wed, Jul 25, 2012 at 05:22:19PM +0800, Xufeng Zhang wrote:
> On 7/25/12, Xufeng Zhang <xufengzhang.main@gmail.com> wrote:
> > On 7/25/12, Vlad Yasevich <vyasevich@gmail.com> wrote:
> >>
> >> Actually not true. AUTH can be before SACK. So can any addip chunks
> >> that
> >> aid in locating an association.
> >>
> >> Now AUTH isn't a big issue since its autogenerated to the packet but
> >> ADDIP
> >> is since it could be queued up for retransmission.
> >>
> >> There could be other extensions as well. It really needs to be done
> >> either
> >> through side effects or making error chunks go at the end of other
> >> control
> >> chunks. Need to audit the spec to see if that's ok.
> > You are right, I just found SHUTDOWN chunks are also before SACK based on
> > your commit "[SCTP]: Fix SACK sequence during shutdown".
> > Maybe the only solution is to do some work on side effects just as you
> > said.
> > Thanks for your explanation!
>
> And after take a moment to look into the relative codes, I think we
> can implement it
> by below way:
> 1). Add a flag(isi_err_needed) in the embedded struct peer of struct
> struct sctp_association
> just like sack_needed flag.
> 2). When "invalid stream identifier" ERROR happens in sctp_eat_data()
> function, we just
> set isi_err_needed flag and don't create ERROR chunk and also don't
> insert SCTP_CMD_REPLY command.
> 3). In sctp_gen_sack() function, we create ERROR chunk and also insert
> SCTP_CMD_REPLY command if isi_err_needed flag is set.
>
> Is this way proper?
>
That would probably work yes. Another way might just be to do some re-ordering
in sctp_outq_flush. Before processing the control chunk list, scan it, and:
1) move all error chunks to the head of the list
2) move all sack chunks to the head of the list
3) move all shutdown chunks to the head of the list
You can do that in a single iteration of the list if you use a few on-stack
lists and list_splice
Neil
>
> Thanks,
> Xufeng Zhang
> >
> >
> >
> > Thanks,
> > Xufeng Zhang
> >>
> >> -vlad
> >>>
> >>>Thanks,
> >>>Xufeng Zhang
> >>>>
> >>>> -vlad
> >>
> >>
> >> --
> >> Sent from my Android phone with SkitMail. Please excuse my brevity.
> >>
> >
>
^ permalink raw reply
* [PATCH v0 net-next 1/1] net/pch_gpe: Cannot disable ethernet autonegation
From: w90p710 @ 2012-07-25 12:13 UTC (permalink / raw)
To: davem; +Cc: netdev, Wei Yang
From: Wei Yang <w90p710@gmail.com>
When attempting to disable ethernet autonegation via ethtool,
the pch_gpe driver will set software reset bit of PHY chip, But
control register of PHY chip of FRI2 will reenable ethernet autonegation.
Signed-off-by: Wei Yang <w90p710@gmail.com>
---
.../ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c | 1 -
.../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 4 +++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
index ac4e72d..e2be4a7 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c
@@ -129,7 +129,6 @@ static int pch_gbe_set_settings(struct net_device *netdev,
hw->mac.link_duplex = ecmd->duplex;
hw->phy.autoneg_advertised = ecmd->advertising;
hw->mac.autoneg = ecmd->autoneg;
- pch_gbe_hal_phy_sw_reset(hw);
/* reset the link */
if (netif_running(adapter->netdev)) {
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 3787c64..8e6d2aa 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -1988,6 +1988,7 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
void pch_gbe_down(struct pch_gbe_adapter *adapter)
{
struct net_device *netdev = adapter->netdev;
+ struct pci_dev *pdev = adapter->pdev;
struct pch_gbe_rx_ring *rx_ring = adapter->rx_ring;
/* signal that we're down so the interrupt handler does not
@@ -2004,7 +2005,8 @@ void pch_gbe_down(struct pch_gbe_adapter *adapter)
netif_carrier_off(netdev);
netif_stop_queue(netdev);
- pch_gbe_reset(adapter);
+ if ((pdev->error_state) && (pdev->error_state != pci_channel_io_normal))
+ pch_gbe_reset(adapter);
pch_gbe_clean_tx_ring(adapter, adapter->tx_ring);
pch_gbe_clean_rx_ring(adapter, adapter->rx_ring);
--
1.7.9.5
^ permalink raw reply related
* RE: [PATCH] be2net: Missing byteswap in be_get_fw_log_level causes oops on PowerPC
From: Sathya.Perla @ 2012-07-25 12:18 UTC (permalink / raw)
To: anton, subbu.seetharaman, Ajit.Khaparde, netdev
In-Reply-To: <20120725110525.0468f754@kryten>
>-----Original Message-----
>From: Anton Blanchard [mailto:anton@samba.org]
>
>We are seeing an oops in be_get_fw_log_level on ppc64 where we walk
>off the end of memory.
>
>commit 941a77d582c8 (be2net: Fix to allow get/set of debug levels in
>the firmware.) requires byteswapping of num_modes and num_modules.
>
>Cc: stable@vger.kernel.org # 3.5+
>Signed-off-by: Anton Blanchard <anton@samba.org>
Acked-by: Sathya Perla <sperla@emulex.com>
Thanks for the fix!
>---
>
>diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c
>b/drivers/net/ethernet/emulex/benet/be_ethtool.c
>index 63e51d4..59ee51a 100644
>--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
>+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
>@@ -910,8 +910,9 @@ static void be_set_fw_log_level(struct be_adapter *adapter,
>u32 level)
> if (!status) {
> cfgs = (struct be_fat_conf_params *)(extfat_cmd.va +
> sizeof(struct be_cmd_resp_hdr));
>- for (i = 0; i < cfgs->num_modules; i++) {
>- for (j = 0; j < cfgs->module[i].num_modes; j++) {
>+ for (i = 0; i < le32_to_cpu(cfgs->num_modules); i++) {
>+ u32 num_modes = le32_to_cpu(cfgs-
>>module[i].num_modes);
>+ for (j = 0; j < num_modes; j++) {
> if (cfgs->module[i].trace_lvl[j].mode ==
> MODE_UART)
> cfgs->module[i].trace_lvl[j].dbg_lvl =
>diff --git a/drivers/net/ethernet/emulex/benet/be_main.c
>b/drivers/net/ethernet/emulex/benet/be_main.c
>index 501dfa9..bd5cf7e 100644
>--- a/drivers/net/ethernet/emulex/benet/be_main.c
>+++ b/drivers/net/ethernet/emulex/benet/be_main.c
>@@ -3479,7 +3479,7 @@ u32 be_get_fw_log_level(struct be_adapter *adapter)
> if (!status) {
> cfgs = (struct be_fat_conf_params *)(extfat_cmd.va +
> sizeof(struct be_cmd_resp_hdr));
>- for (j = 0; j < cfgs->module[0].num_modes; j++) {
>+ for (j = 0; j < le32_to_cpu(cfgs->module[0].num_modes); j++) {
> if (cfgs->module[0].trace_lvl[j].mode == MODE_UART)
> level = cfgs->module[0].trace_lvl[j].dbg_lvl;
> }
^ permalink raw reply
* Re: [PATCH net-next] netns: correctly use per-netns ipv4 sysctl_tcp_mem
From: Glauber Costa @ 2012-07-25 12:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Huang Qiang, David Miller, netdev, containers, yangzhenzhang
In-Reply-To: <1342677832.2626.3839.camel@edumazet-glaptop>
Hi,
On 07/19/2012 10:03 AM, Eric Dumazet wrote:
> On Thu, 2012-07-19 at 13:38 +0800, Huang Qiang wrote:
>> From: Yang Zhenzhang <yangzhenzhang@huawei.com>
>>
>> Now, kernel allows each net namespace to independently set up its levels
>> for tcp memory pressure thresholds.
Not really.
So the real limitation here, is done by the memory controller in cgroup,
not the proc files. AFAIK, lxc does not (yet) touches that file by
default, but it does create a memcg placeholder for you container, where
you can set that yourself.
cgroups are outside the realm of the admin, however. So once the
limitation is in place, you might want to restrain their further,
and that's the role of the files in /proc.
The goal is to have something that is as close as possible to a real
system in a container, where an admin could freely set this. (but of
course, never going over its allowance)
You can note this by what reads in sysctl_ipv4.c, when that file is
written to:
#ifdef CONFIG_MEMCG_KMEM
rcu_read_lock();
memcg = mem_cgroup_from_task(current);
tcp_prot_mem(memcg, vec[0], 0);
tcp_prot_mem(memcg, vec[1], 1);
tcp_prot_mem(memcg, vec[2], 2);
rcu_read_unlock();
#endif
This function is defined in tcp_memcontrol.c
void tcp_prot_mem(struct mem_cgroup *memcg, long val, int idx)
{
struct tcp_memcontrol *tcp;
struct cg_proto *cg_proto;
cg_proto = tcp_prot.proto_cgroup(memcg);
if (!cg_proto)
return;
tcp = tcp_from_cgproto(cg_proto);
tcp->tcp_prot_mem[idx] = val;
}
tcp_prot_mem[] ends up being the vector you access as:
prot = sk->sk_cgrp->sysctl_mem;
in the function you patch.
I hope it helps.
^ permalink raw reply
* open sockets preventing unregister_netdevice from completing in linux-next (next-20120724)
From: Bjørn Mork @ 2012-07-25 13:45 UTC (permalink / raw)
To: netdev
I am currently researching several power management regressions in
linux-next as of next-20120724, spread over the PCI, USB and net
subsystems. This one I believe belongs to the net subsystem, although I
definitely may be wrong, mixing these together.
My test case is:
- open a ssh connection over a USB network device (qmi_wwan - which is why
I am looking at this, but I really don't think it's the driver this time)
- suspend laptop with netdev and ssh connection up
- attempt to resume
The USB device will be gone on resume because it is power cycled, so the
drivers need to clean up, to let the device be rediscovered and bound
again. But this does not happen anymore in linux-next as long as some
socket is open. Instead we have these messages:
Jul 25 15:13:11 nemi kernel: [ 7704.560306] unregister_netdevice: waiting for wwan0 to become free. Usage count = 1
Jul 25 15:13:21 nemi kernel: [ 7714.800308] unregister_netdevice: waiting for wwan0 to become free. Usage count = 1
Jul 25 15:13:31 nemi kernel: [ 7725.040316] unregister_netdevice: waiting for wwan0 to become free. Usage count = 1
There are quite a few problems with the system in this state. Any write
to the power/control associated with that USB device will hang,
presumably because the USB device does not exist anymore. This will
also make new attempts to suspend fail. And the USB device is of course
not functional. The driver has not yet had a chance to clean up any of
the other devices associated with the dead USB device (wwan1,
/dev/cdc-wdm0 and /dev/cdc-wdm1), so these ghost devices will appear as
non- functional. And new devices cannot be registered until the
previous USB device is deleted and a new one created.
Killing the ssh session let the unregister_netdevice continue and
everything will be cleaned up and go back to normal.
This is a regression compared to 3.5, where unregister_netdevice would
succeed regardless of any open sockets. Or maybe the sockets were
auto-reaped? I don't know the inner details - just observing the
results.
The test case above is quite normal operational mode for me. I often
leave open sessions while suspending (because I intend to continue using
them after resuming). And I always forget that this won't work for the
USB modem case. I don't really care either. I expect the netdev to be
removed, routes deleted and any sockets referencing either should just
die or live on as zombies or whatever. The important part is that they
should not prevent deletion of a netdev when e.g. the physical device is
gone. That's the way things used to work.
Bjørn
^ permalink raw reply
* re: bonding: sync netpoll code with bridge
From: Dan Carpenter @ 2012-07-25 13:47 UTC (permalink / raw)
To: amwang; +Cc: netdev
Hello Amerigo Wang,
The patch 8a8efa22f51b: "bonding: sync netpoll code with bridge" from
Feb 17, 2011, leads to the following warning:
drivers/net/bonding/bond_main.c:1849 bond_enslave()
error: scheduling with locks held: 'read_lock:&bond->lock'
drivers/net/bonding/bond_main.c
1301 read_lock(&bond->lock);
1302 bond_for_each_slave(bond, slave, i) {
1303 err = slave_enable_netpoll(slave);
^^^^^^^^^^^^^^^^^^^^^^^^^^^
This can lead to a scheduling while atomic bug because it does a
GFP_KERNEL allocation and calls __netpoll_setup() which sleeps.
1304 if (err) {
1305 __bond_netpoll_cleanup(bond);
1306 break;
1307 }
1308 }
1309 read_unlock(&bond->lock);
Also later in the file:
1848 if (slave_dev->npinfo) {
1849 if (slave_enable_netpoll(new_slave)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We are holding read_lock(&bond->lock);
1850 read_unlock(&bond->lock);
1851 pr_info("Error, %s: master_dev is using netpoll, "
1852 "but new slave device does not support netpoll.\n",
1853 bond_dev->name);
1854 res = -EBUSY;
1855 goto err_detach;
1856 }
The easy way to trigger this warning is to test with
CONFIG_DEBUG_ATOMIC_SLEEP=y.
regards,
dan carpenter
^ permalink raw reply
* RE: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
From: KY Srinivasan @ 2012-07-25 14:10 UTC (permalink / raw)
To: Ben Hutchings
Cc: olaf@aepfle.de, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org,
netdev@vger.kernel.org, apw@canonical.com,
devel@linuxdriverproject.org
In-Reply-To: <1343178644.5132.103.camel@deadeye.wl.decadent.org.uk>
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Tuesday, July 24, 2012 9:11 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; olaf@aepfle.de;
> apw@canonical.com; netdev@vger.kernel.org
> Subject: Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
>
> On Tue, 2012-07-24 at 09:01 -0700, K. Y. Srinivasan wrote:
> > In preparation to implementing IP injection, cleanup the way we propagate
> > and handle errors both in the driver as well as in the user level daemon.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > drivers/hv/hv_kvp.c | 112 +++++++++++++++++++++++++++++++++++++-
> --------
> > include/linux/hyperv.h | 17 +++++---
> > tools/hv/hv_kvp_daemon.c | 70 +++++++++++++++-------------
> > 3 files changed, 138 insertions(+), 61 deletions(-)
> >
> > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> > index 0012eed..9b7fc4a 100644
> > --- a/drivers/hv/hv_kvp.c
> > +++ b/drivers/hv/hv_kvp.c
> [...]
> > @@ -109,27 +154,52 @@ kvp_cn_callback(struct cn_msg *msg, struct
> netlink_skb_parms *nsp)
> > {
> > struct hv_kvp_msg *message;
> > struct hv_kvp_msg_enumerate *data;
> > + int error = 0;
> >
> > message = (struct hv_kvp_msg *)msg->data;
> > - switch (message->kvp_hdr.operation) {
> > +
> > + /*
> > + * If we are negotiating the version information
> > + * with the daemon; handle that first.
> > + */
> > +
> > + if (in_hand_shake) {
> > + if (kvp_handle_handshake(message))
> > + in_hand_shake = false;
> > + return;
> > + }
> > +
> > + /*
> > + * Based on the version of the daemon, we propagate errors from the
> > + * daemon differently.
> > + */
> > +
> > + data = &message->body.kvp_enum_data;
> > +
> > + switch (dm_reg_value) {
> > case KVP_OP_REGISTER:
> > - pr_info("KVP: user-mode registering done.\n");
> > - kvp_register();
> > - kvp_transaction.active = false;
> > - hv_kvp_onchannelcallback(kvp_transaction.kvp_context);
> > + /*
> > + * Null string is used to pass back error condition.
> > + */
> > + if (!strlen(data->data.key))
>
> Do we know that the key is null-terminated here? Shouldn't we just
> check whether data->data.key[0] == 0?
Yes, currently we do return null string to indicate error.
>
> > + error = HV_S_CONT;
> > break;
> >
> > - default:
> > - data = &message->body.kvp_enum_data;
> > + case KVP_OP_REGISTER1:
> > /*
> > - * Complete the transaction by forwarding the key value
> > - * to the host. But first, cancel the timeout.
> > + * We use the message header information from
> > + * the user level daemon to transmit errors.
> > */
> > - if (cancel_delayed_work_sync(&kvp_work))
> > - kvp_respond_to_host(data->data.key,
> > - data->data.value,
> > - !strlen(data->data.key));
> > + error = *((int *)(&message->kvp_hdr.operation));
> [...]
>
> What's with the casting (repeated in many other places)? Wouldn't it be
> better to redefine struct hv_kvp_msg to start with something like:
>
> union {
> struct hv_kvp_hdr request;
> int error;
> } kvp_hdr;
Agreed; will do.
>
> Ben.
>
> --
> Ben Hutchings
> If more than one person is responsible for a bug, no one is at fault.
^ permalink raw reply
* RE: [PATCH 08/17] Tools: hv: Gather subnet information
From: KY Srinivasan @ 2012-07-25 14:10 UTC (permalink / raw)
To: Ben Hutchings
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
olaf@aepfle.de, apw@canonical.com, netdev@vger.kernel.org
In-Reply-To: <1343178850.5132.104.camel@deadeye.wl.decadent.org.uk>
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Tuesday, July 24, 2012 9:14 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; olaf@aepfle.de;
> apw@canonical.com; netdev@vger.kernel.org
> Subject: Re: [PATCH 08/17] Tools: hv: Gather subnet information
>
> On Tue, 2012-07-24 at 09:01 -0700, K. Y. Srinivasan wrote:
> > Now gather sub-net information for the specified interface.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > tools/hv/hv_kvp_daemon.c | 31 +++++++++++++++++++++++++++++--
> > 1 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
> > index 79eb130..2c24ebf 100644
> > --- a/tools/hv/hv_kvp_daemon.c
> > +++ b/tools/hv/hv_kvp_daemon.c
> > @@ -534,6 +534,7 @@ kvp_get_ip_address(int family, char *if_name, int op,
> > struct ifaddrs *ifap;
> > struct ifaddrs *curp;
> > int offset = 0;
> > + int sn_offset = 0;
> > const char *str;
> > int error = 0;
> > char *buffer;
> > @@ -594,12 +595,38 @@ kvp_get_ip_address(int family, char *if_name, int op,
> > * Gather info other than the IP address.
> > * IP address info will be gathered later.
> > */
> > - if (curp->ifa_addr->sa_family == AF_INET)
> > + if (curp->ifa_addr->sa_family == AF_INET) {
> > ip_buffer->addr_family |= ADDR_FAMILY_IPV4;
> > - else
> > + /*
> > + * Get subnet info.
> > + */
> > + error = kvp_process_ip_address(
> > + curp->ifa_netmask,
> > + AF_INET,
> > + (char *)
> > + ip_buffer->sub_net,
> > + length,
> > + &sn_offset);
> [...]
>
> This is barely readable; why don't you indent the arguments by just one
> extra tab?
Will do.
Regards,
K. Y
^ permalink raw reply
* [PATCH net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
From: Or Gerlitz @ 2012-07-25 14:35 UTC (permalink / raw)
To: davem; +Cc: netdev, oren, Amir Vadai, Ben Hutchings, Or Gerliz
From: Amir Vadai <amirv@mellanox.com>
RFS filter id can't have the special value RPS_NO_FILTER,
need to skip it when allocating id's.
Also, changed an ifdef into a more elegant IS_DEFINED.
CC: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
Signed-off-by: Or Gerliz <ogerlitz@mellanox.com>
---
Addressing feedback from Ben Hutchings
drivers/net/ethernet/mellanox/mlx4/en_cq.c | 8 ++------
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
index aa9c2f6..866829b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
struct mlx4_en_dev *mdev = priv->mdev;
int err = 0;
char name[25];
- struct cpu_rmap *rmap =
-#ifdef CONFIG_RFS_ACCEL
- priv->dev->rx_cpu_rmap;
-#else
- NULL;
-#endif
+ struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
+ priv->dev->rx_cpu_rmap : NULL;
cq->dev = mdev->pndev[priv->port];
cq->mcq.set_ci_db = cq->wqres.db.db;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 8864d8b..edd9cb8 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -201,7 +201,7 @@ mlx4_en_filter_alloc(struct mlx4_en_priv *priv, int rxq_index, __be32 src_ip,
filter->flow_id = flow_id;
- filter->id = priv->last_filter_id++;
+ filter->id = priv->last_filter_id++ % RPS_NO_FILTER;
list_add_tail(&filter->next, &priv->filters);
hlist_add_head(&filter->filter_chain,
--
1.7.8.2
^ permalink raw reply related
* [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
From: Or Gerlitz @ 2012-07-25 14:36 UTC (permalink / raw)
To: davem; +Cc: netdev, oren, Amir Vadai, Ben Hutchings, Or Gerlitz
From: Amir Vadai <amirv@mellanox.com>
RFS filter id can't have the special value RPS_NO_FILTER,
need to skip it when allocating id's.
Also, changed an ifdef into a more elegant IS_DEFINED.
CC: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
Addressing feedback from Ben Hutchings
resending as of typo in my signature... sorry
drivers/net/ethernet/mellanox/mlx4/en_cq.c | 8 ++------
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
index aa9c2f6..866829b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
struct mlx4_en_dev *mdev = priv->mdev;
int err = 0;
char name[25];
- struct cpu_rmap *rmap =
-#ifdef CONFIG_RFS_ACCEL
- priv->dev->rx_cpu_rmap;
-#else
- NULL;
-#endif
+ struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
+ priv->dev->rx_cpu_rmap : NULL;
cq->dev = mdev->pndev[priv->port];
cq->mcq.set_ci_db = cq->wqres.db.db;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 8864d8b..edd9cb8 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -201,7 +201,7 @@ mlx4_en_filter_alloc(struct mlx4_en_priv *priv, int rxq_index, __be32 src_ip,
filter->flow_id = flow_id;
- filter->id = priv->last_filter_id++;
+ filter->id = priv->last_filter_id++ % RPS_NO_FILTER;
list_add_tail(&filter->next, &priv->filters);
hlist_add_head(&filter->filter_chain,
--
1.7.8.2
^ permalink raw reply related
* Re: [PATCH] mlx4: Add support for EEH error recovery
From: Shlomo Pongartz @ 2012-07-25 14:38 UTC (permalink / raw)
To: Or Gerlitz
Cc: David Miller, klebers, netdev, jackm, yevgenyp, ogerlitz,
cascardo, brking
In-Reply-To: <CAJZOPZKUw7DQQH5mFq4XK5GOZ_Wj6JmjFYi6no7aL_UrpO10vw@mail.gmail.com>
On 7/25/2012 1:30 AM, Or Gerlitz wrote:
> On Wed, Jul 25, 2012 at 12:03 AM, David Miller <davem@davemloft.net> wrote:
>
>> Or, you promised an ACK today, I still haven't seen it.
> It turned out that reacted we did, but not the ACK way.
>
> Again, code review wise, we intended to ack it, but Shlomo has set
> testing environment, under which he had some issues with the patch, as
> such he preferred not to ACK it but rather bring up the issues on the
> list and sort them out 1st. I thought it would be wrong to over-rule
> this preference of him, and this way is fair-enough with the author
> and your guide-lines, maybe I had to be more aggressive with ACKing
> this, as of the merge window closing coming. So tomorrow.
>
> Or.
>
>> There is no reason Kleber should be penalized and have his work
>> miss the merge window just because you guys can't be bothered
>> to approve this patch in a reasonable amount of time.
>>
>> Therefore I'm just going to apply it later today, and don't do this
>> with someone's submission ever again, it impedes progress and
>> frustrates contributors.
> .
>
The code looks fine and can be merged.
We will continue to test it under various conditions.
Sorry for the delay.
Thank you.
Shlomo Pongratz.
^ permalink raw reply
* Re: open sockets preventing unregister_netdevice from completing in linux-next (next-20120724)
From: Eric Dumazet @ 2012-07-25 14:38 UTC (permalink / raw)
To: Bjørn Mork; +Cc: netdev
In-Reply-To: <87boj4hs3k.fsf@nemi.mork.no>
On Wed, 2012-07-25 at 15:45 +0200, Bjørn Mork wrote:
> I am currently researching several power management regressions in
> linux-next as of next-20120724, spread over the PCI, USB and net
> subsystems. This one I believe belongs to the net subsystem, although I
> definitely may be wrong, mixing these together.
>
> My test case is:
>
> - open a ssh connection over a USB network device (qmi_wwan - which is why
> I am looking at this, but I really don't think it's the driver this time)
> - suspend laptop with netdev and ssh connection up
> - attempt to resume
>
> The USB device will be gone on resume because it is power cycled, so the
> drivers need to clean up, to let the device be rediscovered and bound
> again. But this does not happen anymore in linux-next as long as some
> socket is open. Instead we have these messages:
>
> Jul 25 15:13:11 nemi kernel: [ 7704.560306] unregister_netdevice: waiting for wwan0 to become free. Usage count = 1
> Jul 25 15:13:21 nemi kernel: [ 7714.800308] unregister_netdevice: waiting for wwan0 to become free. Usage count = 1
> Jul 25 15:13:31 nemi kernel: [ 7725.040316] unregister_netdevice: waiting for wwan0 to become free. Usage count = 1
>
>
> There are quite a few problems with the system in this state. Any write
> to the power/control associated with that USB device will hang,
> presumably because the USB device does not exist anymore. This will
> also make new attempts to suspend fail. And the USB device is of course
> not functional. The driver has not yet had a chance to clean up any of
> the other devices associated with the dead USB device (wwan1,
> /dev/cdc-wdm0 and /dev/cdc-wdm1), so these ghost devices will appear as
> non- functional. And new devices cannot be registered until the
> previous USB device is deleted and a new one created.
>
> Killing the ssh session let the unregister_netdevice continue and
> everything will be cleaned up and go back to normal.
>
> This is a regression compared to 3.5, where unregister_netdevice would
> succeed regardless of any open sockets. Or maybe the sockets were
> auto-reaped? I don't know the inner details - just observing the
> results.
>
> The test case above is quite normal operational mode for me. I often
> leave open sessions while suspending (because I intend to continue using
> them after resuming). And I always forget that this won't work for the
> USB modem case. I don't really care either. I expect the netdev to be
> removed, routes deleted and any sockets referencing either should just
> die or live on as zombies or whatever. The important part is that they
> should not prevent deletion of a netdev when e.g. the physical device is
> gone. That's the way things used to work.
>
>
Yes, we miss what was done with rt_cache_flush() : find all cached
routes and release all dev references...
^ permalink raw reply
* Re: [PATCH] ppp: add 64 bit stats
From: Kevin Groeneveld @ 2012-07-25 14:43 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1343189061.2626.11073.camel@edumazet-glaptop>
On Wed, Jul 25, 2012 at 12:04 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> It there are spinlocks already, why even adding u64_stats_sync ?
That is a good question I have already wondered about myself.
>include/linux/u64_stats_sync.h
>
>* 3) Write side must ensure mutual exclusion or one seqcount update could
> * be lost, thus blocking readers forever.
> * If this synchronization point is not a mutex, but a spinlock or
> * spinlock_bh() or disable_bh() :
It seems the u64_stats_sync requires some form of mutual exclusion.
So why bother ever using it at all? Maybe there are cases where the
required mutual exclusion can be cheaper than a spinlock? Maybe it is
just to avoid the spinlocks on the read side of things?
I hope you don't mind all my questions...
^ permalink raw reply
* Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
From: Ben Hutchings @ 2012-07-25 14:47 UTC (permalink / raw)
To: KY Srinivasan
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
olaf@aepfle.de, apw@canonical.com, netdev@vger.kernel.org
In-Reply-To: <426367E2313C2449837CD2DE46E7EAF9236A8B61@SN2PRD0310MB382.namprd03.prod.outlook.com>
On Wed, Jul 25, 2012 at 02:10:05PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Ben Hutchings [mailto:ben@decadent.org.uk]
> > Sent: Tuesday, July 24, 2012 9:11 PM
> > To: KY Srinivasan
> > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; olaf@aepfle.de;
> > apw@canonical.com; netdev@vger.kernel.org
> > Subject: Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
> >
> > On Tue, 2012-07-24 at 09:01 -0700, K. Y. Srinivasan wrote:
> > > In preparation to implementing IP injection, cleanup the way we propagate
> > > and handle errors both in the driver as well as in the user level daemon.
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > ---
> > > drivers/hv/hv_kvp.c | 112 +++++++++++++++++++++++++++++++++++++-
> > --------
> > > include/linux/hyperv.h | 17 +++++---
> > > tools/hv/hv_kvp_daemon.c | 70 +++++++++++++++-------------
> > > 3 files changed, 138 insertions(+), 61 deletions(-)
> > >
> > > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> > > index 0012eed..9b7fc4a 100644
> > > --- a/drivers/hv/hv_kvp.c
> > > +++ b/drivers/hv/hv_kvp.c
> > [...]
> > > @@ -109,27 +154,52 @@ kvp_cn_callback(struct cn_msg *msg, struct
> > netlink_skb_parms *nsp)
> > > {
> > > struct hv_kvp_msg *message;
> > > struct hv_kvp_msg_enumerate *data;
> > > + int error = 0;
> > >
> > > message = (struct hv_kvp_msg *)msg->data;
> > > - switch (message->kvp_hdr.operation) {
> > > +
> > > + /*
> > > + * If we are negotiating the version information
> > > + * with the daemon; handle that first.
> > > + */
> > > +
> > > + if (in_hand_shake) {
> > > + if (kvp_handle_handshake(message))
> > > + in_hand_shake = false;
> > > + return;
> > > + }
> > > +
> > > + /*
> > > + * Based on the version of the daemon, we propagate errors from the
> > > + * daemon differently.
> > > + */
> > > +
> > > + data = &message->body.kvp_enum_data;
> > > +
> > > + switch (dm_reg_value) {
> > > case KVP_OP_REGISTER:
> > > - pr_info("KVP: user-mode registering done.\n");
> > > - kvp_register();
> > > - kvp_transaction.active = false;
> > > - hv_kvp_onchannelcallback(kvp_transaction.kvp_context);
> > > + /*
> > > + * Null string is used to pass back error condition.
> > > + */
> > > + if (!strlen(data->data.key))
> >
> > Do we know that the key is null-terminated here? Shouldn't we just
> > check whether data->data.key[0] == 0?
>
> Yes, currently we do return null string to indicate error.
[...]
So the kernel should assume userland input is always valid?
Ben.
--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus
^ permalink raw reply
* RE: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
From: KY Srinivasan @ 2012-07-25 14:48 UTC (permalink / raw)
To: Ben Hutchings
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
olaf@aepfle.de, apw@canonical.com, netdev@vger.kernel.org
In-Reply-To: <1343179491.5132.112.camel@deadeye.wl.decadent.org.uk>
Ben,
At the outset I want to thank you for taking the time to review this code. Given that Greg
has indicated that he will not be able to look at this patch set till 3.6 and the nature of review
comments I have gotten from you and others, I will re-spin this patch set to address all the
comments I have gotten to date. Specific responses to your comments are in-line.
Regards,
K. Y
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Tuesday, July 24, 2012 9:25 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; olaf@aepfle.de;
> apw@canonical.com; netdev@vger.kernel.org
> Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> KVP_OP_SET_IP_INFO
>
> On Tue, 2012-07-24 at 09:01 -0700, K. Y. Srinivasan wrote:
> > Implement the KVP verb - KVP_OP_SET_IP_INFO. This operation configures
> the
> > specified interface based on the given configuration. Since configuring
> > an interface is very distro specific, we invoke an external script to
> > configure the interface.
> [...]
> > +static int kvp_write_file(FILE *f, char *s1, char *s2, char *s3)
> > +{
> > + char str[256];
> > + int error;
> > +
> > + memset(str, 0, sizeof(str));
> > + strcat(str, s1);
> > + if (s2 != NULL)
> > + strcat(str, s2);
> > + strcat(str, "=");
> > + strcat(str, s3);
> > + strcat(str, "\n");
> > +
> > + error = fputs(str, f);
>
> This style of string pasting is crazy; have you never heard of
> fprintf()?
>
> [...]
> > + /*
> > + * Set the configuration for the specified interface with
> > + * the information provided. Since there is no standard
> > + * way to configure an interface, we will have an external
> > + * script that does the job of configuring the interface and
> > + * flushing the configuration.
> > + *
> > + * The parameters passed to this external script are:
> > + * 1. A configuration file that has the specified configuration.
> > + *
> > + * We will embed the name of the interface in the configuration
> > + * file: ifcfg-ethx (where ethx is the interface name).
> > + *
> > + * Here is the format of the ip configuration file:
> > + *
> > + * HWADDR=macaddr
>
> Is the interface supposed to be matched by name or by MAC address?
I do not dictate that. My plan was to package all the information I have about
the interface and the desired configuration in a file and invoke the external
distro specific script to do its magic. This external script is free to ignore
what it does not need.
>
> > + * BOOTPROTO=dhcp (dhcp enabled for the interface)
>
> The BOOTPROTO line may or may not appear.
>
> > + * NM_CONTROLLED=no (this interface will not be controlled by NM)
> > + * PEERDNS=yes
>
> I wonder what the point is of including constant lines in the file.
> What is the external script supposed to do if it these apparent
> constants change in future?
As you can see, I did my testing on a RHEL system and I was too lazy to create
a RHEL specific config file in the external script and so I ended up creating pretty much
the config file needed by RHEL in this daemon. All the external script had to do was to
simply copy this file into the right location and bring up the interface. So, if you prefer
that we not populate the config file with these constant lines, I can get rid of them.
As I noted earlier external scripts may not choose to use all the information in the file
that this daemon generates. What I have done here, simplifies the external script
at least for one distro.
>
> > + * IPADDR_x=ipaddr
> > + * NETMASK_x=netmask
> > + * GATEWAY_x=gateway
> > + * DNSx=dns
>
> A strangely familiar format...
>
> > + * IPV6 addresses will be tagged as IPV6ADDR, IPV6 gateway will be
> > + * tagged as IPV6_DEFAULTGW and IPV6 NETMASK will be tagged as
> > + * IPV6NETMASK.
> > + */
> > +
> > + memset(if_file, 0, sizeof(if_file));
> > + strcat(if_file, "/var/opt/hyperv/ifcfg-");
>
> Like I said before about the key-value files, this should be under
> /var/lib if the daemon is included in a distribution. You should
> perhaps use a macro for the "/var/opt" part so it can be overridden
> depending on whether it's built as a distribution or add-on package.
I will make this a macro.
>
> > + strcat(if_file, if_name);
> > +
> > + file = fopen(if_file, "w");
> > +
> > + if (file == NULL) {
> > + syslog(LOG_ERR, "Failed to open config file");
> > + return HV_E_FAIL;
> > + }
> > +
> > + /*
> > + * First write out the MAC address.
> > + */
> > +
> > + mac_addr = kvp_if_name_to_mac(if_name);
> > + if (mac_addr == NULL) {
> > + error = HV_E_FAIL;
> > + goto setval_error;
> > + }
> > +
> > + error = kvp_write_file(file, "HWADDR", NULL, mac_addr);
> > + if (error)
> > + goto setval_error;
> > +
> > + error = kvp_write_file(file, "ONBOOT", NULL, "yes");
> > + if (error)
> > + goto setval_error;
> > +
> > + error = kvp_write_file(file, "IPV6INIT", NULL, "yes");
> > + if (error)
> > + goto setval_error;
> [...]
>
> This line isn't mentioned in the above comment.
>
> Ben.
>
> --
> Ben Hutchings
> If more than one person is responsible for a bug, no one is at fault.
^ permalink raw reply
* Re: [PATCH] ppp: add 64 bit stats
From: Eric Dumazet @ 2012-07-25 14:50 UTC (permalink / raw)
To: Kevin Groeneveld; +Cc: netdev
In-Reply-To: <CABF+-6VAWVM=Cpyjk4iTbG-bBfHUs_CfRhe7rXmxxiF_ZX7_OA@mail.gmail.com>
On Wed, 2012-07-25 at 10:43 -0400, Kevin Groeneveld wrote:
> On Wed, Jul 25, 2012 at 12:04 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > It there are spinlocks already, why even adding u64_stats_sync ?
>
> That is a good question I have already wondered about myself.
>
> >include/linux/u64_stats_sync.h
> >
> >* 3) Write side must ensure mutual exclusion or one seqcount update could
> > * be lost, thus blocking readers forever.
> > * If this synchronization point is not a mutex, but a spinlock or
> > * spinlock_bh() or disable_bh() :
>
> It seems the u64_stats_sync requires some form of mutual exclusion.
> So why bother ever using it at all? Maybe there are cases where the
> required mutual exclusion can be cheaper than a spinlock? Maybe it is
> just to avoid the spinlocks on the read side of things?
>
> I hope you don't mind all my questions...
u64_stats_sync is good for write sides without any locking,
for example using percpu data.
So if you use shared counters, u64_stats_sync has no value.
^ permalink raw reply
* RE: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
From: KY Srinivasan @ 2012-07-25 14:51 UTC (permalink / raw)
To: Ben Hutchings
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
olaf@aepfle.de, apw@canonical.com, netdev@vger.kernel.org
In-Reply-To: <20120725144724.GD1894@decadent.org.uk>
> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Wednesday, July 25, 2012 10:47 AM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; olaf@aepfle.de;
> apw@canonical.com; netdev@vger.kernel.org
> Subject: Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
>
> On Wed, Jul 25, 2012 at 02:10:05PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ben Hutchings [mailto:ben@decadent.org.uk]
> > > Sent: Tuesday, July 24, 2012 9:11 PM
> > > To: KY Srinivasan
> > > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > > devel@linuxdriverproject.org; virtualization@lists.osdl.org; olaf@aepfle.de;
> > > apw@canonical.com; netdev@vger.kernel.org
> > > Subject: Re: [PATCH 03/17] Drivers: hv: kvp: Cleanup error handling in KVP
> > >
> > > On Tue, 2012-07-24 at 09:01 -0700, K. Y. Srinivasan wrote:
> > > > In preparation to implementing IP injection, cleanup the way we propagate
> > > > and handle errors both in the driver as well as in the user level daemon.
> > > >
> > > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > ---
> > > > drivers/hv/hv_kvp.c | 112
> +++++++++++++++++++++++++++++++++++++-
> > > --------
> > > > include/linux/hyperv.h | 17 +++++---
> > > > tools/hv/hv_kvp_daemon.c | 70 +++++++++++++++-------------
> > > > 3 files changed, 138 insertions(+), 61 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
> > > > index 0012eed..9b7fc4a 100644
> > > > --- a/drivers/hv/hv_kvp.c
> > > > +++ b/drivers/hv/hv_kvp.c
> > > [...]
> > > > @@ -109,27 +154,52 @@ kvp_cn_callback(struct cn_msg *msg, struct
> > > netlink_skb_parms *nsp)
> > > > {
> > > > struct hv_kvp_msg *message;
> > > > struct hv_kvp_msg_enumerate *data;
> > > > + int error = 0;
> > > >
> > > > message = (struct hv_kvp_msg *)msg->data;
> > > > - switch (message->kvp_hdr.operation) {
> > > > +
> > > > + /*
> > > > + * If we are negotiating the version information
> > > > + * with the daemon; handle that first.
> > > > + */
> > > > +
> > > > + if (in_hand_shake) {
> > > > + if (kvp_handle_handshake(message))
> > > > + in_hand_shake = false;
> > > > + return;
> > > > + }
> > > > +
> > > > + /*
> > > > + * Based on the version of the daemon, we propagate errors from the
> > > > + * daemon differently.
> > > > + */
> > > > +
> > > > + data = &message->body.kvp_enum_data;
> > > > +
> > > > + switch (dm_reg_value) {
> > > > case KVP_OP_REGISTER:
> > > > - pr_info("KVP: user-mode registering done.\n");
> > > > - kvp_register();
> > > > - kvp_transaction.active = false;
> > > > - hv_kvp_onchannelcallback(kvp_transaction.kvp_context);
> > > > + /*
> > > > + * Null string is used to pass back error condition.
> > > > + */
> > > > + if (!strlen(data->data.key))
> > >
> > > Do we know that the key is null-terminated here? Shouldn't we just
> > > check whether data->data.key[0] == 0?
> >
> > Yes, currently we do return null string to indicate error.
> [...]
>
> So the kernel should assume userland input is always valid?
Good point! This is the existing code and this patch-set cleans up all
the error handling and would not have this problem.
K. Y
^ permalink raw reply
* Re: [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
From: Ben Hutchings @ 2012-07-25 14:57 UTC (permalink / raw)
To: Or Gerlitz; +Cc: davem, netdev, oren, Amir Vadai
In-Reply-To: <1343226979-32004-1-git-send-email-ogerlitz@mellanox.com>
On Wed, 2012-07-25 at 17:36 +0300, Or Gerlitz wrote:
> From: Amir Vadai <amirv@mellanox.com>
>
> RFS filter id can't have the special value RPS_NO_FILTER,
> need to skip it when allocating id's.
>
> Also, changed an ifdef into a more elegant IS_DEFINED.
>
> CC: Ben Hutchings <bhutchings@solarflare.com>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
>
> Addressing feedback from Ben Hutchings
>
> resending as of typo in my signature... sorry
--signoff is so much easier than typing it every time :-)
> drivers/net/ethernet/mellanox/mlx4/en_cq.c | 8 ++------
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
> 2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> index aa9c2f6..866829b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> @@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
> struct mlx4_en_dev *mdev = priv->mdev;
> int err = 0;
> char name[25];
> - struct cpu_rmap *rmap =
> -#ifdef CONFIG_RFS_ACCEL
> - priv->dev->rx_cpu_rmap;
> -#else
> - NULL;
> -#endif
> + struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
> + priv->dev->rx_cpu_rmap : NULL;
This is a separate change.
> cq->dev = mdev->pndev[priv->port];
> cq->mcq.set_ci_db = cq->wqres.db.db;
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 8864d8b..edd9cb8 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -201,7 +201,7 @@ mlx4_en_filter_alloc(struct mlx4_en_priv *priv, int rxq_index, __be32 src_ip,
>
> filter->flow_id = flow_id;
>
> - filter->id = priv->last_filter_id++;
> + filter->id = priv->last_filter_id++ % RPS_NO_FILTER;
This should do for now.
Ideally the filter ID would be based on the index used in hardware. We
would need to change the API to allow for devices where this cannot be
determined synchronously, as in this driver. (And the filter ID might
also need to be wider.)
Ben.
>
> list_add_tail(&filter->next, &priv->filters);
> hlist_add_head(&filter->filter_chain,
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
From: Ben Hutchings @ 2012-07-25 14:57 UTC (permalink / raw)
To: Or Gerlitz; +Cc: davem, netdev, oren, Amir Vadai
In-Reply-To: <1343226979-32004-1-git-send-email-ogerlitz@mellanox.com>
On Wed, 2012-07-25 at 17:36 +0300, Or Gerlitz wrote:
> From: Amir Vadai <amirv@mellanox.com>
>
> RFS filter id can't have the special value RPS_NO_FILTER,
> need to skip it when allocating id's.
>
> Also, changed an ifdef into a more elegant IS_DEFINED.
>
> CC: Ben Hutchings <bhutchings@solarflare.com>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
>
> Addressing feedback from Ben Hutchings
>
> resending as of typo in my signature... sorry
--signoff is so much easier than typing it every time :-)
> drivers/net/ethernet/mellanox/mlx4/en_cq.c | 8 ++------
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
> 2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> index aa9c2f6..866829b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
> @@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
> struct mlx4_en_dev *mdev = priv->mdev;
> int err = 0;
> char name[25];
> - struct cpu_rmap *rmap =
> -#ifdef CONFIG_RFS_ACCEL
> - priv->dev->rx_cpu_rmap;
> -#else
> - NULL;
> -#endif
> + struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
> + priv->dev->rx_cpu_rmap : NULL;
This ought to be a separate change really.
> cq->dev = mdev->pndev[priv->port];
> cq->mcq.set_ci_db = cq->wqres.db.db;
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 8864d8b..edd9cb8 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -201,7 +201,7 @@ mlx4_en_filter_alloc(struct mlx4_en_priv *priv, int rxq_index, __be32 src_ip,
>
> filter->flow_id = flow_id;
>
> - filter->id = priv->last_filter_id++;
> + filter->id = priv->last_filter_id++ % RPS_NO_FILTER;
This should do for now.
Ideally the filter ID would be based on the index used in hardware. We
would need to change the API to allow for devices where this cannot be
determined synchronously, as in this driver. (And the filter ID might
also need to be wider.)
Ben.
>
> list_add_tail(&filter->next, &priv->filters);
> hlist_add_head(&filter->filter_chain,
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Vlad Yasevich @ 2012-07-25 15:00 UTC (permalink / raw)
To: Xufeng Zhang
Cc: Neil Horman, xufeng zhang, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CA+=dFzgXq3Sx-Ny+6oDZ3BEdsMQCgswNEVPVnhmhedHFvazzWw@mail.gmail.com>
On 07/25/2012 05:22 AM, Xufeng Zhang wrote:
> On 7/25/12, Xufeng Zhang <xufengzhang.main@gmail.com> wrote:
>> On 7/25/12, Vlad Yasevich <vyasevich@gmail.com> wrote:
>>>
>>> Actually not true. AUTH can be before SACK. So can any addip chunks
>>> that
>>> aid in locating an association.
>>>
>>> Now AUTH isn't a big issue since its autogenerated to the packet but
>>> ADDIP
>>> is since it could be queued up for retransmission.
>>>
>>> There could be other extensions as well. It really needs to be done
>>> either
>>> through side effects or making error chunks go at the end of other
>>> control
>>> chunks. Need to audit the spec to see if that's ok.
>> You are right, I just found SHUTDOWN chunks are also before SACK based on
>> your commit "[SCTP]: Fix SACK sequence during shutdown".
>> Maybe the only solution is to do some work on side effects just as you
>> said.
>> Thanks for your explanation!
>
> And after take a moment to look into the relative codes, I think we
> can implement it
> by below way:
> 1). Add a flag(isi_err_needed) in the embedded struct peer of struct
> struct sctp_association
> just like sack_needed flag.
> 2). When "invalid stream identifier" ERROR happens in sctp_eat_data()
> function, we just
> set isi_err_needed flag and don't create ERROR chunk and also don't
> insert SCTP_CMD_REPLY command.
> 3). In sctp_gen_sack() function, we create ERROR chunk and also insert
> SCTP_CMD_REPLY command if isi_err_needed flag is set.
>
> Is this way proper?
>
So, I looked at the code, and it looks very simple to do. We already
return a specific status from sctp_eat_data() when the error was
generated. All you have to do is take the code that generates the error
and adds it to the command list and give it its own small function that
you can then call if SCTP_IERROR_BAD_STREAM error was returned.
-vlad
>
> Thanks,
> Xufeng Zhang
>>
>>
>>
>> Thanks,
>> Xufeng Zhang
>>>
>>> -vlad
>>>>
>>>> Thanks,
>>>> Xufeng Zhang
>>>>>
>>>>> -vlad
>>>
>>>
>>> --
>>> Sent from my Android phone with SkitMail. Please excuse my brevity.
>>>
>>
^ permalink raw reply
* Re: [PATCH] mlx4: Add support for EEH error recovery
From: Kleber Sacilotto de Souza @ 2012-07-25 15:02 UTC (permalink / raw)
To: Shlomo Pongartz
Cc: Or Gerlitz, David Miller, netdev, jackm, yevgenyp, ogerlitz,
cascardo, brking
In-Reply-To: <5010070B.5040405@mellanox.com>
On 07/25/2012 11:47 AM, Shlomo Pongartz wrote:
> On 7/25/2012 1:30 AM, Or Gerlitz wrote:
>> On Wed, Jul 25, 2012 at 12:03 AM, David Miller <davem@davemloft.net> wrote:
>>
>>> Or, you promised an ACK today, I still haven't seen it.
>> It turned out that reacted we did, but not the ACK way.
>>
>> Again, code review wise, we intended to ack it, but Shlomo has set
>> testing environment, under which he had some issues with the patch, as
>> such he preferred not to ACK it but rather bring up the issues on the
>> list and sort them out 1st. I thought it would be wrong to over-rule
>> this preference of him, and this way is fair-enough with the author
>> and your guide-lines, maybe I had to be more aggressive with ACKing
>> this, as of the merge window closing coming. So tomorrow.
>>
>> Or.
>>
>>> There is no reason Kleber should be penalized and have his work
>>> miss the merge window just because you guys can't be bothered
>>> to approve this patch in a reasonable amount of time.
>>>
>>> Therefore I'm just going to apply it later today, and don't do this
>>> with someone's submission ever again, it impedes progress and
>>> frustrates contributors.
>> .
>>
>
> Hi Kleber,
>
>
>
> I reviewed the patch and it seems fine, here's my ACK
>
>
>
> Acked-by: Shlomo Pongratz <shlomop@mellanox.com
> <mailto:shlomop@mellanox.com>>
>
>
Hi Shlomo,
Thank you for reviewing my patch.
--
Kleber Sacilotto de Souza
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH RESEND net] net/mlx4_en: Limit the RFS filter IDs to be < RPS_NO_FILTER
From: Or Gerlitz @ 2012-07-25 15:04 UTC (permalink / raw)
To: Ben Hutchings; +Cc: davem, netdev, oren, Amir Vadai
In-Reply-To: <1343228230.2694.12.camel@bwh-desktop.uk.solarflarecom.com>
On 25/07/2012 17:57, Ben Hutchings wrote:
>
> resending as of typo in my signature... sorry
>
> --signoff is so much easier than typing it every time :-)
I know and I do that on the regular basis, today did that differently
and here's the result...
Or.
>
>
>> drivers/net/ethernet/mellanox/mlx4/en_cq.c | 8 ++------
>> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
>> 2 files changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
>> index aa9c2f6..866829b 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
>> @@ -77,12 +77,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
>> struct mlx4_en_dev *mdev = priv->mdev;
>> int err = 0;
>> char name[25];
>> - struct cpu_rmap *rmap =
>> -#ifdef CONFIG_RFS_ACCEL
>> - priv->dev->rx_cpu_rmap;
>> -#else
>> - NULL;
>> -#endif
>> + struct cpu_rmap *rmap = IS_ENABLED(CONFIG_RFS_ACCEL) ?
>> + priv->dev->rx_cpu_rmap : NULL;
>
> This is a separate change.
OK, will send two patches
>
>
>> cq->dev = mdev->pndev[priv->port];
>> cq->mcq.set_ci_db = cq->wqres.db.db;
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> index 8864d8b..edd9cb8 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> @@ -201,7 +201,7 @@ mlx4_en_filter_alloc(struct mlx4_en_priv *priv, int rxq_index, __be32 src_ip,
>>
>> filter->flow_id = flow_id;
>>
>> - filter->id = priv->last_filter_id++;
>> + filter->id = priv->last_filter_id++ % RPS_NO_FILTER;
>
> This should do for now.
thanks
Or.
>
>
> Ideally the filter ID would be based on the index used in hardware. We
> would need to change the API to allow for devices where this cannot be
> determined synchronously, as in this driver. (And the filter ID might
> also need to be wider.)
>
> Ben.
>
>>
>> list_add_tail(&filter->next,&priv->filters);
>> hlist_add_head(&filter->filter_chain,
>
^ 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