* Re: [PATCH net-next] sockopt: Introduce the SO_BINDTOIFINDEX
From: Brian Haley @ 2012-10-23 21:43 UTC (permalink / raw)
To: David Miller; +Cc: xemul, bhutchings, eric.dumazet, netdev
In-Reply-To: <20121023.133602.1844873902881466397.davem@davemloft.net>
On 10/23/2012 01:36 PM, David Miller wrote:
>
> This is really a huge ball of confusion.
>
> The user asks for a device to bind to, and they want the device which
> had a particular name at the time of the call.
>
> If you allow setting this via ifindex, the issues and races are the
> same whether the name-->ifindex translation is done before the
> sockopt() call or during it.
Yes, there are race conditions in the socket APIs, even IP addresses can go away
right before a bind() call.
> Furthermore both the name and the ifindex can change (that latter
> via module unload/load).
>
> If you want me to consider these changes seriously, talk less about
> obtuse issues like symmetry and more about what problems it actually
> solves. As far as I can tell, all the same real issues still exist
> even if we had this new interface.
Noone's complained about not having this getsockopt() for over 7+ years, so in
that sense I'm not sure what problem it solved adding it.
> In fact, I wouldn't mind if a getsockopt() on SO_BINDTODEVICE returned
> BOTH the name and the ifindex in a special structure. Then you could
> actually construct a more foolproof mechanism on the user side to try
> various ways to get the same device bound to during restart.
There is no fool-proof way to do any of this since we can agree device names and
indexes can change. But people aren't typically running daemons listening on
interfaces that are constantly changing like taps or tunnels, instead it's on
eth1 so they can run a private DHCP server.
> Symmetry is over-rated.
If any of the other SO_* options didn't set and get using the same structure I'd
agree with you, but I can't find any that do.
-Brian
^ permalink raw reply
* Re: [PATCH net-next] sockopt: Make SO_BINDTODEVICE readable
From: Brian Haley @ 2012-10-23 21:43 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Pavel Emelyanov, Linux Netdev List, David Miller
In-Reply-To: <1350942738.8609.1375.camel@edumazet-glaptop>
On 10/22/2012 05:52 PM, Eric Dumazet wrote:
>>> I meant another cpu can be changing dev->name[] content while the
>>> strcpy() is done, and you get a mangled devname, like "for" or "bao"
>>> instead of "foo" or "bar"
>>
>> Even when holding the rcu_read_lock()? I'd have to hold the rtnl lock there?
>
> Yes, rcu_read_lock() only makes sure the device doesnt disappear.
>
> But its name can be changed.
There's a similar bug in the SIOCGIFNAME/dev_ifname() code too then, but I would
think this rare enough that it doesn't happen in practice.
-Brian
^ permalink raw reply
* Synchronization between process context and softirq context on SMP machine
From: Cong Xu @ 2012-10-23 20:56 UTC (permalink / raw)
To: netdev
I met some problems when I did some research in improving the TCP/UDP
performance of Virtual Machine(VM), if anybody can offer me some help or
suggestion to handle my problem, I will be very appreciated.
On virtual machine platform, virtual CPU (vCPU) of each VM can not be always
online when several vCPUs share one physical CPU (pCPU) (Here we can simply
assume the vCPU scheduling is round-robin.). Therefore, the high delay of TCP
receiving of VM hurts the TCP throughput significantly. In order to handle this
problem I assign a virtual co-processor (co-vCPU) which is almost always online
to each VM and pin NIC IRQ of the VM to this co-vCPU. ( If you are not familiar
with VM, you can simply assume that in a common OS the user level application
(e.g. iperf) runs on a cpu which will be offline every 30ms, and the bottom half
or softirq context runs on another cpu which is always online. )
In my experiment, this method works well for UDP but does not work for TCP. I
doubt that it is due to the synchronization between process context and softirq
context. Because when I read some source code of TCP layer in linux, I found
that both softirq context (e.g. tcp_v4_rcv() in net/ipv4/tcp_ipv4.c) and process
context (e.g. tcp_recvmsg() in net/ipv4/tcp.c) call lock_sock()/unlock_sock()
when they access the buffers in kernel(receive_queue, backlog_queue or p
requeue). Therefore, sometimes softirq context can not access the receiving
buffers locked by another vCPU which runs the user level receiving process
(iperf server) and this vCPU holding the spinlock has been descheduled by VM
monitor(VMM) or hypervisor.
I am not sure I described my problem clearly. Anyway, welcome any suggestion on
it.
^ permalink raw reply
* 555555555555 /*2300000000000000000000000000000..
From: xuchuangkyzd @ 2012-10-23 20:30 UTC (permalink / raw)
To: info, jinhui_2001, kptychmq
±¾¹«Ë¾×¨Òµ°ìÈ«¹ú¸÷µØ£¨¹ú¡¢µØ£©·¢////Ʊ£¬
Õæαȷ±££¬Ñéºó¸¶¿î£¡Ë°ÂʵÍÁ®£¡
\
13632608856¡¾ÐìÏÈÉú¡¿Q Q: ¡¾QQ¡¿917519835
^ permalink raw reply
* Re: [PATCH v2 net-next 2/2] r8169: update the settings for RTL8111G
From: Francois Romieu @ 2012-10-23 19:41 UTC (permalink / raw)
To: Hayes Wang; +Cc: netdev, linux-kernel
In-Reply-To: <1350974831-1438-2-git-send-email-hayeswang@realtek.com>
Hayes Wang <hayeswang@realtek.com> :
> Update the parameters of RTL8111G
I am Jack's Broken Heart. :o/
In a not too distant future somebody may try to figure if things should
be fed into one of the numerous -stable trees. He will appreciate figuring
from the comment if this commit is supposed to fix anything or if it's a
pure performance thing or whatever.
The pll_power changes could be a bugfix for instance. The ALDPS change
isn't.
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
> drivers/net/ethernet/realtek/r8169.c | 118 ++++++++++++++++++++++++++++++-----
> 1 file changed, 101 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index cdd46de..3a393f7 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
[...]
> @@ -3400,29 +3410,57 @@ static void rtl8168g_1_hw_phy_config(struct rtl8169_private *tp)
> {
> static const u16 mac_ocp_patch[] = {
> 0xe008, 0xe01b, 0xe01d, 0xe01f,
> - 0xe021, 0xe023, 0xe025, 0xe027,
> + 0xe022, 0xe025, 0xe031, 0xe04d,
> 0x49d2, 0xf10d, 0x766c, 0x49e2,
> 0xf00a, 0x1ec0, 0x8ee1, 0xc60a,
> -
> 0x77c0, 0x4870, 0x9fc0, 0x1ea0,
> 0xc707, 0x8ee1, 0x9d6c, 0xc603,
> 0xbe00, 0xb416, 0x0076, 0xe86c,
> - 0xc602, 0xbe00, 0x0000, 0xc602,
> -
> - 0xbe00, 0x0000, 0xc602, 0xbe00,
> - 0x0000, 0xc602, 0xbe00, 0x0000,
> - 0xc602, 0xbe00, 0x0000, 0xc602,
> - 0xbe00, 0x0000, 0xc602, 0xbe00,
> -
> - 0x0000, 0x0000, 0x0000, 0x0000
> + 0xc602, 0xbe00, 0xa000, 0xc602,
> + 0xbe00, 0x0000, 0x1b76, 0xc202,
> + 0xba00, 0x059c, 0x1b76, 0xc602,
> + 0xbe00, 0x065a, 0x74e6, 0x1b78,
> + 0x46dc, 0x1300, 0xf005, 0x74f8,
> + 0x48c3, 0x48c4, 0x8cf8, 0x64e7,
> + 0xc302, 0xbb00, 0x06a0, 0x74e4,
> + 0x49c5, 0xf106, 0x49c6, 0xf107,
> + 0x48c8, 0x48c9, 0xe011, 0x48c9,
> + 0x4848, 0xe00e, 0x4848, 0x49c7,
> + 0xf00a, 0x48c9, 0xc60d, 0x1d1f,
> + 0x8dc2, 0x1d00, 0x8dc3, 0x1d11,
> + 0x8dc0, 0xe002, 0x4849, 0x94e5,
> + 0xc602, 0xbe00, 0x01f0, 0xe434,
> + 0x49d9, 0xf01b, 0xc31e, 0x7464,
> + 0x49c4, 0xf114, 0xc31b, 0x6460,
> + 0x14fa, 0xfa02, 0xe00f, 0xc317,
> + 0x7460, 0x49c0, 0xf10b, 0xc311,
> + 0x7462, 0x48c1, 0x9c62, 0x4841,
> + 0x9c62, 0xc30a, 0x1c04, 0x8c60,
> + 0xe004, 0x1c15, 0xc305, 0x8c60,
> + 0xc602, 0xbe00, 0x0384, 0xe434,
> + 0xe030, 0xe61c, 0xe906
> };
Do you remember why firmware loading was introduced in the first place ?
I find it increasingly hard to believe that it does not apply here.
[...]
> /* Patch code for GPHY reset */
> + r8168_mac_ocp_write(tp, 0xfc26, 0x0000);
> + r8168_mac_ocp_write(tp, 0xfc28, 0x0000);
> + r8168_mac_ocp_write(tp, 0xfc2a, 0x0000);
> + r8168_mac_ocp_write(tp, 0xfc2c, 0x0000);
> + r8168_mac_ocp_write(tp, 0xfc2e, 0x0000);
> + r8168_mac_ocp_write(tp, 0xfc30, 0x0000);
> + r8168_mac_ocp_write(tp, 0xfc32, 0x0000);
> + r8168_mac_ocp_write(tp, 0xfc34, 0x0000);
> + r8168_mac_ocp_write(tp, 0xfc36, 0x0000);
> for (i = 0; i < ARRAY_SIZE(mac_ocp_patch); i++)
> - r8168_mac_ocp_write(tp, 0xf800 + 2*i, mac_ocp_patch[i]);
> + r8168_mac_ocp_write(tp, 0xf800 + 2 * i, mac_ocp_patch[i]);
> r8168_mac_ocp_write(tp, 0xfc26, 0x8000);
> r8168_mac_ocp_write(tp, 0xfc28, 0x0075);
> + r8168_mac_ocp_write(tp, 0xfc2e, 0x059b);
> + r8168_mac_ocp_write(tp, 0xfc30, 0x0659);
> + r8168_mac_ocp_write(tp, 0xfc32, 0x069f);
> + r8168_mac_ocp_write(tp, 0xfc34, 0x01cd);
> + r8168_mac_ocp_write(tp, 0xfc36, 0x0303);
Within a few weeks / months there will be a new update with a new
set of completely obscure parameters. It will takes a kernel cycle or
more to be distributed.
There should be some higher level data struct that could use a mundane
"for" loop and some indirection in place of this code bloat.
It is getting out of control.
--
Ueimor
^ permalink raw reply
* Re: [Pv-drivers] 3.7-rc2 regression : file copied to CIFS-mounted directory corrupted
From: Eric Dumazet @ 2012-10-23 19:39 UTC (permalink / raw)
To: Shreyas Bhatewara
Cc: VMware, Inc., netdev, edumazet, linux-kernel, jongman heo
In-Reply-To: <1351000246.8609.1926.camel@edumazet-glaptop>
On Tue, 2012-10-23 at 15:50 +0200, Eric Dumazet wrote:
> Only the skb head is handled in the code you copy/pasted.
>
> You need to generalize that to code in lines ~754
>
>
> Then, the number of estimated descriptors is bad :
>
> /* conservatively estimate # of descriptors to use */
> count = VMXNET3_TXD_NEEDED(skb_headlen(skb)) +
> skb_shinfo(skb)->nr_frags + 1;
>
>
> Yes, you need a more precise estimation and vmxnet3_map_pkt() should
> eventually split too big frags.
raw patch would be :
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index ce9d4f2..0ae1bcc 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -744,28 +744,43 @@ vmxnet3_map_pkt(struct sk_buff *skb, struct vmxnet3_tx_ctx *ctx,
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
const struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[i];
+ u32 buf_size;
- tbi = tq->buf_info + tq->tx_ring.next2fill;
- tbi->map_type = VMXNET3_MAP_PAGE;
- tbi->dma_addr = skb_frag_dma_map(&adapter->pdev->dev, frag,
- 0, skb_frag_size(frag),
- DMA_TO_DEVICE);
+ buf_offset = 0;
+ len = skb_frag_size(frag);
+ while (len) {
+ tbi = tq->buf_info + tq->tx_ring.next2fill;
+ if (len < VMXNET3_MAX_TX_BUF_SIZE) {
+ buf_size = len;
+ dw2 |= len;
+ } else {
+ buf_size = VMXNET3_MAX_TX_BUF_SIZE;
+ /* spec says that for TxDesc.len, 0 == 2^14 */
+ }
+ tbi->map_type = VMXNET3_MAP_PAGE;
+ tbi->dma_addr = skb_frag_dma_map(&adapter->pdev->dev, frag,
+ buf_offset, buf_size,
+ DMA_TO_DEVICE);
- tbi->len = skb_frag_size(frag);
+ tbi->len = buf_size;
- gdesc = tq->tx_ring.base + tq->tx_ring.next2fill;
- BUG_ON(gdesc->txd.gen == tq->tx_ring.gen);
+ gdesc = tq->tx_ring.base + tq->tx_ring.next2fill;
+ BUG_ON(gdesc->txd.gen == tq->tx_ring.gen);
- gdesc->txd.addr = cpu_to_le64(tbi->dma_addr);
- gdesc->dword[2] = cpu_to_le32(dw2 | skb_frag_size(frag));
- gdesc->dword[3] = 0;
+ gdesc->txd.addr = cpu_to_le64(tbi->dma_addr);
+ gdesc->dword[2] = cpu_to_le32(dw2);
+ gdesc->dword[3] = 0;
- dev_dbg(&adapter->netdev->dev,
- "txd[%u]: 0x%llu %u %u\n",
- tq->tx_ring.next2fill, le64_to_cpu(gdesc->txd.addr),
- le32_to_cpu(gdesc->dword[2]), gdesc->dword[3]);
- vmxnet3_cmd_ring_adv_next2fill(&tq->tx_ring);
- dw2 = tq->tx_ring.gen << VMXNET3_TXD_GEN_SHIFT;
+ dev_dbg(&adapter->netdev->dev,
+ "txd[%u]: 0x%llu %u %u\n",
+ tq->tx_ring.next2fill, le64_to_cpu(gdesc->txd.addr),
+ le32_to_cpu(gdesc->dword[2]), gdesc->dword[3]);
+ vmxnet3_cmd_ring_adv_next2fill(&tq->tx_ring);
+ dw2 = tq->tx_ring.gen << VMXNET3_TXD_GEN_SHIFT;
+
+ len -= buf_size;
+ buf_offset += buf_size;
+ }
}
ctx->eop_txd = gdesc;
@@ -886,6 +901,18 @@ vmxnet3_prepare_tso(struct sk_buff *skb,
}
}
+static int txd_estimate(const struct sk_buff *skb)
+{
+ int count = VMXNET3_TXD_NEEDED(skb_headlen(skb)) + 1;
+ int i;
+
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ const struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[i];
+
+ count += VMXNET3_TXD_NEEDED(skb_frag_size(frag));
+ }
+ return count;
+}
/*
* Transmits a pkt thru a given tq
@@ -914,9 +941,7 @@ vmxnet3_tq_xmit(struct sk_buff *skb, struct vmxnet3_tx_queue *tq,
union Vmxnet3_GenericDesc tempTxDesc;
#endif
- /* conservatively estimate # of descriptors to use */
- count = VMXNET3_TXD_NEEDED(skb_headlen(skb)) +
- skb_shinfo(skb)->nr_frags + 1;
+ count = txd_estimate(skb);
ctx.ipv4 = (vlan_get_protocol(skb) == cpu_to_be16(ETH_P_IP));
^ permalink raw reply related
* Re: [net-next 08/12] ixgbe: using is_zero_ether_addr() to simplify the code
From: Joe Perches @ 2012-10-23 19:35 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: davem, Wei Yongjun, netdev, gospo, sassmann
In-Reply-To: <1350969467.2135.11.camel@jtkirshe-mobl>
On Mon, 2012-10-22 at 22:17 -0700, Jeff Kirsher wrote:
> On Mon, 2012-10-22 at 21:45 -0700, Joe Perches wrote:
> > On Mon, 2012-10-22 at 21:26 -0700, Jeff Kirsher wrote:
> > > From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> > > Using is_zero_ether_addr() to simplify the code.
> > []
> > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> > []
> > > @@ -1778,8 +1778,7 @@ s32 ixgbe_validate_mac_addr(u8 *mac_addr)
> > []
> > > - else if (mac_addr[0] == 0 && mac_addr[1] == 0 && mac_addr[2] == 0 &&
> > > - mac_addr[3] == 0 && mac_addr[4] == 0 && mac_addr[5] == 0)
> > > + else if (is_zero_ether_addr(mac_addr))
> >
> > This means the more comprehensive simplifying patch
> > I submitted doesn't apply. Easy to fix up though.
> >
[]
> Want me to
> send you the fixed up patch so that you can confirm my fixes?
Nope, no worries, I'm sure you'll do the right thing.
I just wanted to alert you about it.
^ permalink raw reply
* Re: [PATCH v2 net-next 1/2] r8169: enable ALDPS for power saving
From: Francois Romieu @ 2012-10-23 19:33 UTC (permalink / raw)
To: Hayes Wang; +Cc: netdev, linux-kernel
In-Reply-To: <1350974831-1438-1-git-send-email-hayeswang@realtek.com>
Hayes Wang <hayeswang@realtek.com> :
> Enable ALDPS function to save power when link down. Note that the
> feature should be set after the other PHY settings. And the firmware
> is necessary. Don't enable it without loading the firmware.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
[...]
Please see my just sent answer in yesterday's thread.
> +static void r810x_aldps_disable(struct rtl8169_private *tp)
> +{
> + rtl_writephy(tp, 0x1f, 0x0000);
> + rtl_writephy(tp, 0x18, 0x0310);
> + msleep(100);
> +}
rtl8402_hw_phy_config used a msleep(20). Meguesses it won't hurt, right ?
[...]
> +
> + /* ALDPS enable */
> + r8168_aldps_enable_1(tp);
The functions are literate enough: you can remove the comment.
[...]
> @@ -6391,6 +6431,8 @@ static void rtl8169_net_suspend(struct net_device *dev)
> {
> struct rtl8169_private *tp = netdev_priv(dev);
>
> + tp->features &= ~RTL_FEATURE_EXTENDED;
> +
> if (!netif_running(dev))
> return;
No (as previously stated).
--
Ueimor
^ permalink raw reply
* Re: [PATCH net-next 1/2] r8169: enable ALDPS for power saving
From: Francois Romieu @ 2012-10-23 19:31 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, linux-kernel
In-Reply-To: <D81B36B929404E28AE7007B7E21B9D46@realtek.com.tw>
hayeswang <hayeswang@realtek.com> :
[...]
> The flag is determined if the firmware is loaded. It doesn't mean to enable
> ALDPS. Besides ALDPS, the firmware includes the other features about power
> saving, performance, hw behavior, and so on. Thus, I think it is the extended
> feature. Any suggestion?
RTL_FEATURE_FW_LOADED
[...]
> I don't sure if the firmware code still exists and works after hibernation. I
> clear the flag for safe. It would be set again after loading firmware. It is
> used to make sure to enable ALDPS with firmware.
I don't understand if you want to avoid a coding bug or a hardware problem.
The code only enables ALDPS in the hw_phy_config helpers. Said helpers are
the only places where firmware loading is requested. Loading the firmware
always set RTL_FEATURE_XYZ.
Whatever you are trying to achieve, the change in rtl8169_net_suspend
seem rather useless. Please explain.
(if you want to enforce that ALDPS does nothing when it is not called
right after firmware loading, the ALDPS enable method itself is imho the
natural place to reset the bit).
[...]
> > After this patch the driver would look like:
> > 1. disable ALDPS before setting firmware (unmodified by patch)
> > RTL_GIGA_MAC_VER_29 "RTL8105e"
> > RTL_GIGA_MAC_VER_30 "RTL8105e"
> > RTL_GIGA_MAC_VER_37 "RTL8402"
> > RTL_GIGA_MAC_VER_39 "RTL8106e"
> >
> > 2. apply_firmware (unmodified by patch)
> > RTL_GIGA_MAC_VER_25 "RTL8168d/8111d"
> > RTL_GIGA_MAC_VER_26 "RTL8168d/8111d"
> > RTL_GIGA_MAC_VER_29 "RTL8105e"
> > RTL_GIGA_MAC_VER_30 "RTL8105e"
> > RTL_GIGA_MAC_VER_32 "RTL8168e/8111e"
> > RTL_GIGA_MAC_VER_33 "RTL8168e/8111e"
> > RTL_GIGA_MAC_VER_34 "RTL8168evl/8111evl"
> > RTL_GIGA_MAC_VER_35 "RTL8168f/8111f"
> > RTL_GIGA_MAC_VER_36 "RTL8168f/8111f"
> > RTL_GIGA_MAC_VER_37 "RTL8402"
> > RTL_GIGA_MAC_VER_38 "RTL8411"
> > RTL_GIGA_MAC_VER_39 "RTL8106e"
> > RTL_GIGA_MAC_VER_40 "RTL8168g/8111g"
> >
> > 3. enable ALDPS after firmware
> > RTL_GIGA_MAC_VER_34 "RTL8168evl/8111evl"
> > RTL_GIGA_MAC_VER_35 "RTL8168f/8111f"
> > RTL_GIGA_MAC_VER_36 "RTL8168f/8111f"
> > RTL_GIGA_MAC_VER_37 "RTL8402"
> > RTL_GIGA_MAC_VER_38 "RTL8411"
> > RTL_GIGA_MAC_VER_39 "RTL8106e"
> > RTL_GIGA_MAC_VER_40 "RTL8168g/8111g"
> >
> > The disable/enable ALDPS code is not trivially balanced.
> >
> > Do we exactly perform the required ALDPS operations ? Nothing more,
> > nothing less ?
> >
>
> Because the different design between the giga nic and 10/100M nic, the behavior
> would be different. For example, you couldn't disable the ALDPS of the giga nic
> directly just like the 10/100M (8136 series) one. The giga nic would disable
> ALDPS automatically when loading firmware, but the 8136 series wouldn't.
It would be nice to state these things in the commit message, namely:
- ALDPS should never be enabled for the RTL8105e
- none of the firmware-free chipsets support ALDPS
- neither do the RTL8168d/8111d
--
Ueimor
^ permalink raw reply
* Re: [PATCH net-next] sock-diag: Report shutdown for inet and unix sockets (v2)
From: David Miller @ 2012-10-23 18:58 UTC (permalink / raw)
To: xemul; +Cc: netdev
In-Reply-To: <5086E224.4030103@parallels.com>
From: Pavel Emelyanov <xemul@parallels.com>
Date: Tue, 23 Oct 2012 22:29:56 +0400
> Make it simple -- just put new nlattr with just sk->sk_shutdown bits.
>
> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
That's a lot better, applied, thanks.
^ permalink raw reply
* [PATCH net-next] sock-diag: Report shutdown for inet and unix sockets (v2)
From: Pavel Emelyanov @ 2012-10-23 18:29 UTC (permalink / raw)
To: David Miller, Linux Netdev List
Make it simple -- just put new nlattr with just sk->sk_shutdown bits.
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
---
include/uapi/linux/inet_diag.h | 3 ++-
include/uapi/linux/unix_diag.h | 1 +
net/ipv4/inet_diag.c | 3 +++
net/unix/diag.c | 3 +++
4 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/include/uapi/linux/inet_diag.h b/include/uapi/linux/inet_diag.h
index 8c469af..bbde90f 100644
--- a/include/uapi/linux/inet_diag.h
+++ b/include/uapi/linux/inet_diag.h
@@ -109,9 +109,10 @@ enum {
INET_DIAG_TOS,
INET_DIAG_TCLASS,
INET_DIAG_SKMEMINFO,
+ INET_DIAG_SHUTDOWN,
};
-#define INET_DIAG_MAX INET_DIAG_SKMEMINFO
+#define INET_DIAG_MAX INET_DIAG_SHUTDOWN
/* INET_DIAG_MEM */
diff --git a/include/uapi/linux/unix_diag.h b/include/uapi/linux/unix_diag.h
index b1d2bf1..b8a2494 100644
--- a/include/uapi/linux/unix_diag.h
+++ b/include/uapi/linux/unix_diag.h
@@ -37,6 +37,7 @@ enum {
UNIX_DIAG_ICONS,
UNIX_DIAG_RQLEN,
UNIX_DIAG_MEMINFO,
+ UNIX_DIAG_SHUTDOWN,
UNIX_DIAG_MAX,
};
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 535584c..e5bad82 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -105,6 +105,9 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
r->id.idiag_src[0] = inet->inet_rcv_saddr;
r->id.idiag_dst[0] = inet->inet_daddr;
+ if (nla_put_u8(skb, INET_DIAG_SHUTDOWN, sk->sk_shutdown))
+ goto errout;
+
/* IPv6 dual-stack sockets use inet->tos for IPv4 connections,
* hence this needs to be included regardless of socket family.
*/
diff --git a/net/unix/diag.c b/net/unix/diag.c
index 06748f1..5ac19dc 100644
--- a/net/unix/diag.c
+++ b/net/unix/diag.c
@@ -151,6 +151,9 @@ static int sk_diag_fill(struct sock *sk, struct sk_buff *skb, struct unix_diag_r
sock_diag_put_meminfo(sk, skb, UNIX_DIAG_MEMINFO))
goto out_nlmsg_trim;
+ if (nla_put_u8(skb, UNIX_DIAG_SHUTDOWN, sk->sk_shutdown))
+ goto out_nlmsg_trim;
+
return nlmsg_end(skb, nlh);
out_nlmsg_trim:
--
1.7.6.5
^ permalink raw reply related
* Re: [RFC PATCH v2 2/6] PM / Runtime: introduce pm_runtime_set_memalloc_noio()
From: Alan Stern @ 2012-10-23 18:16 UTC (permalink / raw)
To: Ming Lei
Cc: linux-kernel, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm
In-Reply-To: <CACVXFVN+=XH_f5BmRkXeagTNowz0o0-Pd7GcxCneO0FSq8xqEw@mail.gmail.com>
On Tue, 23 Oct 2012, Ming Lei wrote:
> With the problem of non-SMP-safe bitfields access, the power.lock should
> be held, but that is not enough to prevent children from being probed or
> disconnected. Looks another lock is still needed. I think a global lock
> is OK in the infrequent path.
Agreed.
> Got it, thanks for your detailed explanation.
>
> Looks the problem is worse than above, not only bitfields are affected, the
> adjacent fields might be involved too, see:
>
> http://lwn.net/Articles/478657/
Linus made it clear (in various emails at the time) that the kernel
requires the compiler not to do the sort of things discussed in that
article. But even the restrictions he wanted would not prevent
adjacent bitfields from interfering with each other.
Alan Stern
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* GET BACK TO ME
From: ADMINISTRATOR @ 2012-10-23 17:50 UTC (permalink / raw)
We are glad to inform you on this year SKL online 2012 promo result.For more
details get back to the Remittance manager on : atlantico.desk@accountant.com
^ permalink raw reply
* Re: [PATCH 3/3] sock-diag: Report shutdown for inet and unix sockets
From: Pavel Emelyanov @ 2012-10-23 17:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <20121023.134236.1262093514433122608.davem@davemloft.net>
On 10/23/2012 09:42 PM, David Miller wrote:
> From: Pavel Emelyanov <xemul@parallels.com>
> Date: Tue, 23 Oct 2012 20:28:22 +0400
>
>> +static inline int shutdown_mask2u(int mask)
>> +{
>> + /*
>> + * map
>> + * RCV_SHUTDOWN -> SHUT_RD
>> + * SEND_SHUTDOWN -> SHUT_WR
>> + * SHUTDOWN_MASK -> SHUT_RDWR
>> + */
>> +
>> + return mask - 1;
>> +}
>
> This is horrible.
>
> You're returning "-1" when the socket hasn't been shutdown in any way.
>
> Do this:
>
> 1) Use a '1' based encoding like the kernel codes so that '0' means
> no shutdown, as any sane interface would. That's why we use that
> representation internally.
>
> 2) Get rid of all of this extension crap, and just report this value
> in the pad byte. In older kernels it will just be zero, which is
> fine.
The response message of inet-diag never had any pad bytes, I'll have to
add the new attrtype (the unix-diag response has one, but I plan to add
attrtype there too for uniformity).
Thanks,
Pavel
^ permalink raw reply
* switching network namespace midway
From: rsa @ 2012-10-23 17:49 UTC (permalink / raw)
To: netdev
Assuming I have a tunnel interface where two route lookups are done --
one for innter
packet and the other for outer -- do you see any issues in switching
the network
namespace prior to second route lookup (and restore to the original namespace
after the second lookup is done)?
If so, are there any other calls other than sk_change_net() needed?
thank you
rsa
^ permalink raw reply
* Re: [PATCH 3/3] sock-diag: Report shutdown for inet and unix sockets
From: David Miller @ 2012-10-23 17:42 UTC (permalink / raw)
To: xemul; +Cc: netdev
In-Reply-To: <5086C5A6.7050101@parallels.com>
From: Pavel Emelyanov <xemul@parallels.com>
Date: Tue, 23 Oct 2012 20:28:22 +0400
> +static inline int shutdown_mask2u(int mask)
> +{
> + /*
> + * map
> + * RCV_SHUTDOWN -> SHUT_RD
> + * SEND_SHUTDOWN -> SHUT_WR
> + * SHUTDOWN_MASK -> SHUT_RDWR
> + */
> +
> + return mask - 1;
> +}
This is horrible.
You're returning "-1" when the socket hasn't been shutdown in any way.
Do this:
1) Use a '1' based encoding like the kernel codes so that '0' means
no shutdown, as any sane interface would. That's why we use that
representation internally.
2) Get rid of all of this extension crap, and just report this value
in the pad byte. In older kernels it will just be zero, which is
fine.
^ permalink raw reply
* Re: [PATCH net-next] sockopt: Introduce the SO_BINDTOIFINDEX
From: David Miller @ 2012-10-23 17:36 UTC (permalink / raw)
To: xemul; +Cc: brian.haley, bhutchings, eric.dumazet, netdev
In-Reply-To: <5086A7FD.3020503@parallels.com>
This is really a huge ball of confusion.
The user asks for a device to bind to, and they want the device which
had a particular name at the time of the call.
If you allow setting this via ifindex, the issues and races are the
same whether the name-->ifindex translation is done before the
sockopt() call or during it.
Furthermore both the name and the ifindex can change (that latter
via module unload/load).
If you want me to consider these changes seriously, talk less about
obtuse issues like symmetry and more about what problems it actually
solves. As far as I can tell, all the same real issues still exist
even if we had this new interface.
In fact, I wouldn't mind if a getsockopt() on SO_BINDTODEVICE returned
BOTH the name and the ifindex in a special structure. Then you could
actually construct a more foolproof mechanism on the user side to try
various ways to get the same device bound to during restart.
Symmetry is over-rated.
^ permalink raw reply
* Re: [RFC PATCH net-next 2/3] rtnl/ipv6: use netconf msg to advertise forwarding status
From: David Miller @ 2012-10-23 17:29 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: netdev
In-Reply-To: <1350985789-24966-3-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue, 23 Oct 2012 11:49:48 +0200
> +static int inet6_fill_netconf_devconf(struct sk_buff *skb, int ifindex,
> + struct ipv6_devconf *devconf, u32 portid,
> + u32 seq, int event, unsigned int flags)
> +{
> + struct nlmsghdr *nlh;
> + struct netconfmsg *ncm;
> +
> + nlh = nlmsg_put(skb, portid, seq, event, sizeof(struct netconfmsg),
> + flags);
> + if (nlh == NULL)
> + return -EMSGSIZE;
> +
> + ncm = nlmsg_data(nlh);
> + ncm->ncm_family = AF_INET6;
> +
> + if (nla_put_s32(skb, NETCONFA_IFINDEX, ifindex) < 0) {
> + nlmsg_cancel(skb, nlh);
> + return -EMSGSIZE;
> + }
> + if (nla_put_s32(skb, NETCONFA_FORWARDING, devconf->forwarding) < 0) {
> + nlmsg_cancel(skb, nlh);
> + return -EMSGSIZE;
> + }
> +
> + return nlmsg_end(skb, nlh);
> +}
Don't report values that aren't changing, that's wasteful.
You need to therefore provide a ->get() operation, so that when
a daemon starts up which is interested in these values, it can
fetch the initial state.
That's why you should only report the differences in the events,
rather than unconditionally reporting all the values all the
time, whether they change or not.
^ permalink raw reply
* Re: [net-next 0/8][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2012-10-23 17:28 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1350987887-16161-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 23 Oct 2012 03:24:39 -0700
> This series contains updates to e1000e, igb, ixgbevf and MAINTAINERS.
>
> The following are changes since commit 598e74f32c6dd4b3dd0ed382c889be07a7c6cbc0:
> Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH 3/3] sock-diag: Report shutdown for inet and unix sockets
From: Pavel Emelyanov @ 2012-10-23 17:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Netdev List, David Miller
In-Reply-To: <1351011194.8609.2301.camel@edumazet-glaptop>
On 10/23/2012 08:53 PM, Eric Dumazet wrote:
> On Tue, 2012-10-23 at 20:28 +0400, Pavel Emelyanov wrote:
>> Add ext bits for inet-diag and unix-diag and report sk_shutdown state.
>
>>
>> + if (ext & (1 << (INET_DIAG_SHUTDOWN - 1)))
>> + if (sock_diag_put_shutdown(sk, skb, INET_DIAG_SHUTDOWN))
>> + goto errout;
>> +
>
> I dont feel the need to make this conditional and consume one bit.
OK, but I have one concern about it.
I'll have to add the attrtype for it anyway. And when later we'll add
yet another one, say INET_DIAG_FOO, we will not be able to request
this in such
req->ext |= (1 << (INET_DIAG_FOO - 1));
manner as it's currently done. If that's OK, I will rework the patch.
Thanks,
Pavel
^ permalink raw reply
* Re: [PATCH net-next 1/2] packet: clean up error variable assignments
From: Daniel Borkmann @ 2012-10-23 17:25 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev
In-Reply-To: <1351011952.8609.2334.camel@edumazet-glaptop>
On Tue, Oct 23, 2012 at 7:05 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2012-10-23 at 13:56 +0200, Daniel Borkmann wrote:
>> This patch performs clean-ups of packet's err variables where appropriate.
>> In particular, errnos are *only* assigned in error cases, which saves
>> useless instructions in non-error cases and makes the code more readable
>> in terms of which error type belongs to which evaluated error condition.
>> Also, in some cases an errno was set, but not used until the next assignment.
>
> I see no value in this patch.
>
> Setting err before a test is a common way to handle error cases and
> generates smaller code in linux kernel.
>
> Better live with it than trying to change it ?
>
> err = -ENOMEM;
> match = kzalloc(xxxx);
> if (!match)
> goto error;
>
>
> is smaller (source code & generated code) than :
>
> match = kzalloc(xxxx);
> if (!match) {
> err = -ENOMEM;
> goto error;
> }
>
> An immediate load is basically free, but code size matters.
Good to know, thanks for the feedback Eric!
^ permalink raw reply
* Re: [PATCH net-next 2/2] packet: minor: make if case unlikely
From: Daniel Borkmann @ 2012-10-23 17:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20121023.131253.779417489113821973.davem@davemloft.net>
On Tue, Oct 23, 2012 at 7:12 PM, David Miller <davem@davemloft.net> wrote:
> From: Daniel Borkmann <dxchgb@gmail.com>
> Date: Tue, 23 Oct 2012 13:58:34 +0200
>
>> This patch fixes the character width of max. 80 chars and also marks the
>> if-statement as unlikely since those flags are not supported for the receive
>> path anyway.
>>
>> Signed-off-by: Daniel Borkmann <daniel.borkmann@tik.ee.ethz.ch>
>
> Please, don't submit patches like this.
>
> 80 characters is a soft "guidelline" there is no strict requirement
> for it.
Good to know.
Thanks for the feedback.
^ permalink raw reply
* Re: [PATCH 0/6] kernel parameters: introduce "macaddr" to set mac address
From: David Miller @ 2012-10-23 17:20 UTC (permalink / raw)
To: p.pisati; +Cc: netdev, peppe.cavallaro, kristoffer
In-Reply-To: <1351012533-3524-1-git-send-email-p.pisati@gmail.com>
From: Paolo Pisati <p.pisati@gmail.com>
Date: Tue, 23 Oct 2012 19:15:27 +0200
> In the past drivers grew mechanism to set their own mac address at boot time
> via (usually) a module parameter. Unfortunately every single driver had its
> own naming/implementation and it soon became clear that a generic
> mechanism was needed.
>
> Introduce "macaddr=", a new kernel parameter to set MAC address using
> netdevice ops (and hence being hardware independent).
Sorry, no, no module parameters.
Run time setting is the only reasonable way to set the MAC address
explicitly.
^ permalink raw reply
* Re: [PATCH v3 1/4] net/macb: fix truncate warnings
From: Joachim Eastwood @ 2012-10-23 17:20 UTC (permalink / raw)
To: Lothar Waßmann
Cc: David Miller, David.Laight, bgat, netdev, nicolas.ferre,
hskinnemoen, plagnioj, linux-arm-kernel, egtvedt
In-Reply-To: <20614.34540.248020.471598@ipc1.ka-ro>
On Tue, Oct 23, 2012 at 2:00 PM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> Hi,
>
> David Miller writes:
>> From: "David Laight" <David.Laight@ACULAB.COM>
>> Date: Tue, 23 Oct 2012 09:21:11 +0100
>>
>> >> When building macb on x86_64 the following warnings show up:
>> >> drivers/net/ethernet/cadence/macb.c: In function macb_interrupt:
>> >> drivers/net/ethernet/cadence/macb.c:556:4: warning: large integer implicitly truncated to unsigned type [-Woverflow]
>> > ...
>> >> - macb_writel(bp, IDR, ~0UL);
>> >> + macb_writel(bp, IDR, -1);
>> >
>> > Seems wrong to fix an error with an unsigned value
>> > by using -1.
>>
>> -1 is equally an unsigned value of all 1's and completely legitimate.
>> The correction being made here is one of size not signedness.
>>
> You could use '~0' (without the 'UL').
The reason for choosing '-1', as noted in patch description, is that
it already appears in a couple of other places in the driver. So I
though it would nice to be consistent.
regards
Joachim Eastwood
^ permalink raw reply
* [PATCH 6/6] fec: remove mac address handling as a module parameter
From: Paolo Pisati @ 2012-10-23 17:15 UTC (permalink / raw)
To: netdev; +Cc: Giuseppe Cavallaro, Kristoffer Glembo
In-Reply-To: <1351012533-3524-1-git-send-email-p.pisati@gmail.com>
Signed-off-by: Paolo Pisati <p.pisati@gmail.com>
---
drivers/net/ethernet/freescale/fec.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index c136230..d5bb97d 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -114,10 +114,6 @@ static const struct of_device_id fec_dt_ids[] = {
};
MODULE_DEVICE_TABLE(of, fec_dt_ids);
-static unsigned char macaddr[ETH_ALEN];
-module_param_array(macaddr, byte, NULL, 0);
-MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
-
#if defined(CONFIG_M5272)
/*
* Some hardware gets it MAC address out of local flash memory.
@@ -786,21 +782,16 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
struct fec_platform_data *pdata = fep->pdev->dev.platform_data;
- unsigned char *iap, tmpaddr[ETH_ALEN];
+ unsigned char *iap = NULL, tmpaddr[ETH_ALEN];
/*
* try to get mac address in following order:
- *
- * 1) module parameter via kernel command line in form
- * fec.macaddr=0x00,0x04,0x9f,0x01,0x30,0xe0
*/
- iap = macaddr;
-
#ifdef CONFIG_OF
/*
- * 2) from device tree data
+ * 1) from device tree data
*/
- if (!is_valid_ether_addr(iap)) {
+ {
struct device_node *np = fep->pdev->dev.of_node;
if (np) {
const char *mac = of_get_mac_address(np);
@@ -811,9 +802,9 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
#endif
/*
- * 3) from flash or fuse (via platform data)
+ * 2) from flash or fuse (via platform data)
*/
- if (!is_valid_ether_addr(iap)) {
+ if (iap == NULL || !is_valid_ether_addr(iap)) {
#ifdef CONFIG_M5272
if (FEC_FLASHMAC)
iap = (unsigned char *)FEC_FLASHMAC;
@@ -824,9 +815,9 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
}
/*
- * 4) FEC mac registers set by bootloader
+ * 3) FEC mac registers set by bootloader
*/
- if (!is_valid_ether_addr(iap)) {
+ if (iap == NULL || !is_valid_ether_addr(iap)) {
*((unsigned long *) &tmpaddr[0]) =
be32_to_cpu(readl(fep->hwp + FEC_ADDR_LOW));
*((unsigned short *) &tmpaddr[4]) =
@@ -835,10 +826,6 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
}
memcpy(ndev->dev_addr, iap, ETH_ALEN);
-
- /* Adjust MAC if using macaddr */
- if (iap == macaddr)
- ndev->dev_addr[ETH_ALEN-1] = macaddr[ETH_ALEN-1] + fep->dev_id;
}
/* ------------------------------------------------------------------------- */
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox