* Re: Question about Mellanox FW reporting (incorrect) port types
From: Or Gerlitz @ 2012-10-17 19:22 UTC (permalink / raw)
To: Marcelo Ricardo Leitner, Yevgeny Petrilin
Cc: netdev, Or Gerlitz, Doug Ledford, Yishai Hadas
In-Reply-To: <507EDC5D.4070602@redhat.com>
Marcelo Ricardo Leitner <mleitner@redhat.com> wrote:
>
> [...] So my questions are: is it possible to the firmware report a wrong port type like
> that? Is it somehow configurable by sysadmin (via fw update, ..), can we flip that byte
> or is it a manufacturing issue?
I'm not sure, Yevgeny/Yishai do you have any insights here?
> I can't try upstream driver
why?! netdev is dealing with upstream, isn't it?
Or.
^ permalink raw reply
* Re: [PATCHv3] ipv6: Enable enough of the code to handle GSO when disabled.
From: Eric Dumazet @ 2012-10-17 19:33 UTC (permalink / raw)
To: vyasevic; +Cc: netdev, davem
In-Reply-To: <507F02F1.5010501@redhat.com>
On Wed, 2012-10-17 at 15:11 -0400, Vlad Yasevich wrote:
> On 10/17/2012 12:13 PM, Eric Dumazet wrote:
> > On Wed, 2012-10-17 at 11:46 -0400, Vlad Yasevich wrote:
> >
> >> This patch attempts to solve this by enabling just enough code so GSO
> >> is correctly processed. However, I should point out that if IPv6 is
> >> simply blacklisted or not built for the kernel, this problem will
> >> still persist.
> >
> > So I guess this should be done in a different way ?
> >
> > We currently use a single structure (struct packet_type) to hold
> > pointers to different methods. (The .func() field, and the gso/gro
> > stuff)
> >
> > We probably need to split it in two parts, and make one part linked into
> > kernel, even if CONFIG_IPV6=n, so that GRO/GSO is fully IPv4/IPv6
> > functional.
>
> The thing about this approach is that if there are any other protocols
> that could have to provide their own segmentation functionality, such
> functionality would always have to be part of the kernel. I wasn't sure
> how much I liked that.
Well, an hypervisor probably has to handle IPv6, at least to a certain
extent.
Make this part a module of its own, or statically linked into vmlinux,
instead of adding some stubs in IPv6 'module'
>
> >
> > By the way, do we really need a hash table for this ?
> > It seems we only have IPv4 (ETH_P_IP) and IPv6 (ETH_P_IPV6) to take care
> > of ?
>
> Which hash are you talking about? I didn't add any hashes.
I suspect you didnt really understood what I said then.
packet_type structures are hashed into ptype_base[]
struct list_head *head = &ptype_base[ntohs(type) & PTYPE_HASH_MASK];
Now let say we split the structure into 2 parts.
ptype_base[] would be used for pre GRO/GSO stuff (packet handlers)
So we would need to add a second hash for the GRO/GSO stuff.
^ permalink raw reply
* Re: [PATCH net] netfilter: nf_conntrack: fix rt_gateway checks for h323
From: Julian Anastasov @ 2012-10-17 19:44 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, netdev, zhaojingmin
In-Reply-To: <20121017123957.GA31202@1984>
Hello,
On Wed, 17 Oct 2012, Pablo Neira Ayuso wrote:
> > But if creating expectation is considered harmless
> > then better to use just the rt_nexthop check because
> > checking for subnets is too risky, hosts can use different
> > subnet masks. By this way we reduce the risk to connect
> > internal hosts without expectation.
>
> It creates an expection for the IP parsed as result of the call
> forwarding that will expire after 240 seconds. So it consumes memory
> but it will expire after that time.
>
> I prefer not to modify this code unless someone using the H.323 helper
> reports a problem.
Agreed
> Thanks for your comments. I'll take your patch and extend the
> description a bit to explain the current situation of that chunk of
> code for the history.
Yes, it needs better description. Thanks!
> > > > diff --git a/net/netfilter/nf_conntrack_h323_main.c b/net/netfilter/nf_conntrack_h323_main.c
> > > > index 1b30b0d..962795e 100644
> > > > --- a/net/netfilter/nf_conntrack_h323_main.c
> > > > +++ b/net/netfilter/nf_conntrack_h323_main.c
> > > > @@ -753,7 +753,8 @@ static int callforward_do_filter(const union nf_inet_addr *src,
> > > > flowi4_to_flowi(&fl1), false)) {
> > > > if (!afinfo->route(&init_net, (struct dst_entry **)&rt2,
> > > > flowi4_to_flowi(&fl2), false)) {
> > > > - if (rt1->rt_gateway == rt2->rt_gateway &&
> > > > + if (rt_nexthop(rt1, fl1.daddr) ==
> > > > + rt_nexthop(rt2, fl2.daddr) &&
> > > > rt1->dst.dev == rt2->dst.dev)
> > > > ret = 1;
> > > > dst_release(&rt2->dst);
> > > > --
> > > > 1.7.3.4
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: Question about Mellanox FW reporting (incorrect) port types
From: Marcelo Ricardo Leitner @ 2012-10-17 19:49 UTC (permalink / raw)
To: Or Gerlitz
Cc: Yevgeny Petrilin, netdev, Or Gerlitz, Doug Ledford, Yishai Hadas
In-Reply-To: <CAJZOPZ+T7paqVe12n+sBw4ra8AEWCZ+W2GZ52iJY=9Hrgoxuaw@mail.gmail.com>
On 10/17/2012 04:22 PM, Or Gerlitz wrote:
> Marcelo Ricardo Leitner <mleitner@redhat.com> wrote:
>>
>> [...] So my questions are: is it possible to the firmware report a wrong port type like
>> that? Is it somehow configurable by sysadmin (via fw update, ..), can we flip that byte
>> or is it a manufacturing issue?
>
>
> I'm not sure, Yevgeny/Yishai do you have any insights here?
>
>> I can't try upstream driver
>
> why?! netdev is dealing with upstream, isn't it?
Yes, it is. By upstream I actually mean a non-RHEL kernel/driver. I
tried but so far couldn't reproduce this issue in-house, sorry. My ports
always answer ETH :) So I have to ask for customer to test and then
unfortunately things get complicated..
Thanks,
Marcelo
^ permalink raw reply
* Re: igb mtu change
From: Alexander Duyck @ 2012-10-17 20:09 UTC (permalink / raw)
To: ratheesh kannoth; +Cc: e1000-devel, linux-net, netdev
In-Reply-To: <CAGZFCEHACn8e99vL3hA_ErvmQ=5E4dfEbO7TjEU=92eeUUT7UQ@mail.gmail.com>
On 10/17/2012 07:41 AM, ratheesh kannoth wrote:
> igb_change_mtu change is only changing adapter->rx_ring[0]->rx_buffer_len.
>
> 1) we dont have to change adapter->tx_ring[0]->rx_buffer_len ?
> 2) Is there any way to set different values to
> adapter->tx_ring[0]->rx_buffer_len and
> adapter->rx_ring[0]->rx_buffer_len ?
>
> I am facing a problem because i have a strange requirement....i have
> 2KB bufffer pools ( GFP_DMA ) in kernel.
> When i receive a jumbo pkt of size 9126, five rx_desc should be
> used to recieve paket. So the first rx_desc will have a status error
> bit indicating that it is not
> EOP ( eop ) ....I could easily keep track of my 2KB buffer pools.
>
> But when sending ...i wll collect all 2KB buffers of same jumbo frame
> and dma to tx_ring....
> So i need 1500 on rx_ring[0]->rx_buffer_len and 9126 on
> adapter->tx_ring[0]->rx_buffer_len. .Is there any way ?
>
> Thanks,
> Ratheesh
What version of the igb driver are you using? What you are describing
doesn't sound like the behaviour of the current igb driver.
The current igb driver does receive the frame data into 2K buffers, and
transmits up to MTU size. The only limitation on the size of the frame
you can transmit is the netdev->mtu which the driver doesn't evaluate.
It is evaluated at the network stack level. The igb driver can send up
to 9K frames, in addition it can TSO up to 64K frames and send them in
9K chunks so I am not sure what you are asking.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH] ixgbe: convert to use simple_open()
From: Jeff Kirsher @ 2012-10-17 22:31 UTC (permalink / raw)
To: Wei Yongjun
Cc: jesse.brandeburg, bruce.w.allan, carolyn.wyborny,
donald.c.skidmore, gregory.v.rose, peter.p.waskiewicz.jr,
alexander.h.duyck, john.ronciak, yongjun_wei, e1000-devel, netdev
In-Reply-To: <CAPgLHd9sFav1MKPCdHNjXM37ZEitiJJYFkH_Kv-8LsSr2t4TYA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 608 bytes --]
On Wed, 2012-10-17 at 16:26 +0800, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> This removes an open coded simple_open() function and
> replaces file operations references to the function
> with simple_open() instead.
>
> dpatch engine is used to auto generate this patch.
> (https://github.com/weiyj/dpatch)
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 32
> ++----------------------
> 1 file changed, 2 insertions(+), 30 deletions(-)
Thanks, I will add the patch to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Bug? TCP shutdown behaviour when deleting local IP addresses
From: Chris Friesen @ 2012-10-17 23:01 UTC (permalink / raw)
To: netdev, David Miller, Alexey Kuznetsov, James Morris,
Patrick McHardy, Hideaki YOSHIFUJI
Hi all,
I sent this to the list yesterday (from another address) but didn't get
any responses. Accordingly I'm expanding the receiver list to the
listed maintainers for IPv4/IPv6.
I'm seeing some unexpected (to me, at least) behaviour with local TCP
connections. The scenario goes as follows:
1) create new IP address and assign to eth device
2) TCP server starts listening on that IP address
3) TCP client connects to server
4) remove new IP address
5) kill server with ctrl-C. At this point it appears that because the
address was removed the shutdown message isn't processed properly.
netstat shows the server socket as FIN_WAIT1, but the client socket is
still ESTABLISHED.
6) client writes to the connected socket (this passes with no error)
7) client waits for response from server, and waits forever or until
keepalive expires
A few points:
This was originally seen on 2.6.27, but I've verified it on 2.6.35. I'll
see about trying it on current git. I've got really simple
client/server code if anyone wants to try reproducing.
If we don't remove the address in step 4, then step 5 results in the
server socket going to FIN_WAIT2 and the client socket going to
CLOSE_WAIT and step 7 returns right away with zero bytes.
It seems like the waiting forever behaviour in step 7 might be
legitimate since the address was removed before shutting down the
server, but it also seems like we should be able to do better given that
everything is local. In the "remove IP address" case maybe step 6
should cause some sort of error since the IP address no longer exists?
Incidentally, if we do this sort of scenario with the client and server
on different hosts then we get a "no route to host" error at step 6.
Curious how this is supposed to work...
Chris
--
Chris Friesen
Software Designer
GENBAND
www.genband.com
^ permalink raw reply
* Re: Bug? TCP shutdown behaviour when deleting local IP addresses
From: David Miller @ 2012-10-17 23:27 UTC (permalink / raw)
To: chris.friesen; +Cc: netdev, kuznet, jmorris, kaber, yoshfuji
In-Reply-To: <507F38D4.102@genband.com>
From: Chris Friesen <chris.friesen@genband.com>
Date: Wed, 17 Oct 2012 17:01:40 -0600
> I sent this to the list yesterday (from another address) but didn't
> get any responses. Accordingly I'm expanding the receiver list to the
> listed maintainers for IPv4/IPv6.
It can take more than 24 hours to get a response from people who are
volunteers, and who will reply to your report because they want to
rather than because they are obligated.
Posting it again just irritates such people, and will place your
report at a much lower priority, just FYI.
^ permalink raw reply
* Re: igb mtu change
From: ratheesh kannoth @ 2012-10-18 0:10 UTC (permalink / raw)
To: Alexander Duyck; +Cc: e1000-devel, linux-net, netdev
In-Reply-To: <507F108A.3080308@intel.com>
On Thu, Oct 18, 2012 at 1:39 AM, Alexander Duyck
<alexander.h.duyck@intel.com> wrote:
> The current igb driver does receive the frame data into 2K buffers, and
> transmits up to MTU size. The only limitation on the size of the frame
> you can transmit is the netdev->mtu which the driver doesn't evaluate.
> It is evaluated at the network stack level. The igb driver can send up
> to 9K frames, in addition it can TSO up to 64K frames and send them in
> 9K chunks so I am not sure what you are asking.
>
> Thanks,
>
> Alex
Thanks a lot.
I am using igb-3.3.6 for hardware - vendor=0x8086, device=0x10e6
only CONFIG_IGB_DISABLE_PACKET_SPLIT defined.
I will explain my question in detail.
int i = 0 ;
rx_desc = IGB_RX_DESC(rx_ring, i);
if(!(igb_test_staterr(rx_desc, E1000_RXD_STAT_EOP) ) {
printk ( "part of Jumbo frame ...Not the last junk ");
}
If mtu is 1500, and jumbo frame size is 9200. I get the printk -
part of Jumbo frame ...Not the last junk .
But if i change mtu to 9216, the condition ,
!(igb_test_staterr(rx_desc, E1000_RXD_STAT_EOP) , looks like
never satisfied .
Thanks,
Ratheesh
^ permalink raw reply
* Re: [PATCH] qla3xxx: Ensure req_q_phy_addr writes to the register
From: Joe Jin @ 2012-10-18 2:18 UTC (permalink / raw)
To: Jitendra Kalsaria
Cc: Ron Mercer, Dept-Eng Linux Driver, netdev, linux-kernel,
Greg Marsden
In-Reply-To: <5E4F49720D0BAD499EE1F01232234BA87744A93B8E@AVEXMB1.qlogic.org>
On 10/18/12 01:45, Jitendra Kalsaria wrote:
>
>
>> -----Original Message-----
>> From: Joe Jin [mailto:joe.jin@oracle.com]
>> Sent: Tuesday, October 16, 2012 11:32 PM
>> To: Ron Mercer; Jitendra Kalsaria; Dept-Eng Linux Driver
>> Cc: netdev; linux-kernel; Greg Marsden
>> Subject: [PATCH] qla3xxx: Ensure req_q_phy_addr writes to the register
>>
>> Make sure req_q_phy_addr write to the register.
>>
>> Signed-off-by: Joe Jin <joe.jin@oracle.com>
>> Cc: Ron Mercer <ron.mercer@qlogic.com>
>> Cc: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
>> ---
>> drivers/net/ethernet/qlogic/qla3xxx.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c b/drivers/net/ethernet/qlogic/qla3xxx.c
>> index df09b1c..78b4cba 100644
>> --- a/drivers/net/ethernet/qlogic/qla3xxx.c
>> +++ b/drivers/net/ethernet/qlogic/qla3xxx.c
>> @@ -2525,6 +2525,12 @@ static int ql_alloc_net_req_rsp_queues(struct ql3_adapter *qdev)
>> qdev->req_q_size =
>> (u32) (NUM_REQ_Q_ENTRIES * sizeof(struct ob_mac_iocb_req));
>>
>> + /*
>> + * The barrier is required to ensure that req_q_phy_addr writes to
>> + * the memory.
>> + */
>> + wmb();
>> +
>> qdev->req_q_virt_addr =
>> pci_alloc_consistent(qdev->pdev,
>> (size_t) qdev->req_q_size,
>
> Your changes only take care of request queue but not response queue which also need barrier.
Jiten,
Thanks for review!
The barrier to make sure writel() call for req_q_phy_addr and rsp_q_phy_addr in
ql_adapter_initialize(), so I think call once wmb() is enough but I need to update
the comment, any idea?
Thanks,
Joe
>
> qdev->req_q_size =
> (u32) (NUM_REQ_Q_ENTRIES * sizeof(struct ob_mac_iocb_req));
>
> qdev->rsp_q_size = NUM_RSP_Q_ENTRIES * sizeof(struct net_rsp_iocb);
>
> wmb();
>
> thanks,
> Jiten
>
--
Oracle <http://www.oracle.com>
Joe Jin | Software Development Senior Manager | +8610.6106.5624
ORACLE | Linux and Virtualization
No. 24 Zhongguancun Software Park, Haidian District | 100193 Beijing
^ permalink raw reply
* Re: [PATCH 1/6] drivers: net: ethernet: cpsw: add support for CPSW register offset changes in different IP version
From: Richard Cochran @ 2012-10-18 2:45 UTC (permalink / raw)
To: Mugunthan V N; +Cc: netdev, davem
In-Reply-To: <1350427518-7230-2-git-send-email-mugunthanvnm@ti.com>
On Wed, Oct 17, 2012 at 04:15:13AM +0530, Mugunthan V N wrote:
>
> +#define CPSW_VERSION_1 0x19010a
> +#define CPSW_VERSION_2 0x19010c
Are you sure about these codes? What about 0x19010b?
I could not find them documented anywhere.
> +#define cpsw_slave_reg(priv, slave, reg) \
> + ((slave)->regs + (priv)->slave_reg_ofs[(reg)])
> +
> #define CPSW_MAJOR_VERSION(reg) (reg >> 8 & 0x7)
> #define CPSW_MINOR_VERSION(reg) (reg & 0xff)
> #define CPSW_RTL_VERSION(reg) ((reg >> 11) & 0x1f)
> @@ -117,6 +122,48 @@ do { \
> disable_irq_nosync(priv->irqs_table[i]); \
> } while (0);
>
> +enum CPSW_SLAVE_REG_OFS {
> + MAX_BLKS,
> + BLK_CNT,
> + FLOW_THRESH,
> + PORT_VLAN,
> + TX_PRI_MAP,
> + TS_CTL,
> + TS_SEQ_LTYPE,
> + TS_VLAN,
> + SA_LO,
> + SA_HI,
> + PORT_CONTROL,
> + TS_SEQ_MTYPE,
> + TS_CONTROL,
> +};
> +
> +static const u32 slave_reg_map_ip_v1[] = {
> + [MAX_BLKS] = 0x00,
> + [BLK_CNT] = 0x04,
> + [FLOW_THRESH] = 0x08,
> + [PORT_VLAN] = 0x0c,
> + [TX_PRI_MAP] = 0x10,
> + [TS_CTL] = 0x14,
> + [TS_SEQ_LTYPE] = 0x18,
> + [TS_VLAN] = 0x1c,
> + [SA_LO] = 0x20,
> + [SA_HI] = 0x24,
> +};
> +
> +static const u32 slave_reg_map_ip_v2[] = {
> + [PORT_CONTROL] = 0x00,
> + [TS_CONTROL] = 0x04,
You don't make use of this register in your driver, so what is the
point?
> + [MAX_BLKS] = 0x08,
> + [BLK_CNT] = 0x0c,
> + [FLOW_THRESH] = 0x10,
> + [PORT_VLAN] = 0x14,
> + [TX_PRI_MAP] = 0x18,
> + [TS_SEQ_MTYPE] = 0x1c,
> + [SA_LO] = 0x20,
> + [SA_HI] = 0x24,
> +};
> +
This is wasting memory with unused static stables. There is a better
way to handle this issue.
> static int debug_level;
> module_param(debug_level, int, 0);
> MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
> @@ -146,19 +193,13 @@ struct cpsw_regs {
> u32 soft_reset;
> u32 stat_port_en;
> u32 ptype;
> -};
> -
> -struct cpsw_slave_regs {
> - u32 max_blks;
> - u32 blk_cnt;
> - u32 flow_thresh;
> - u32 port_vlan;
> - u32 tx_pri_map;
> - u32 ts_ctl;
> - u32 ts_seq_ltype;
> - u32 ts_vlan;
> - u32 sa_lo;
> - u32 sa_hi;
> + u32 soft_idle;
> + u32 thru_rate;
> + u32 gap_thresh;
> + u32 tx_start_wds;
> + u32 flow_control;
> + u32 vlan_ltype;
> + u32 ts_ltype;
> };
>
> struct cpsw_host_regs {
> @@ -185,7 +226,7 @@ struct cpsw_sliver_regs {
> };
>
> struct cpsw_slave {
> - struct cpsw_slave_regs __iomem *regs;
> + void __iomem *regs;
> struct cpsw_sliver_regs __iomem *sliver;
> int slave_num;
> u32 mac_control;
> @@ -215,6 +256,8 @@ struct cpsw_priv {
> struct cpdma_ctlr *dma;
> struct cpdma_chan *txch, *rxch;
> struct cpsw_ale *ale;
> + u32 cpsw_version;
> + u32 *slave_reg_ofs;
> /* snapshot of IRQ numbers */
> u32 irqs_table[4];
> u32 num_irqs;
> @@ -359,8 +402,8 @@ static inline void soft_reset(const char *module, void __iomem *reg)
> static void cpsw_set_slave_mac(struct cpsw_slave *slave,
> struct cpsw_priv *priv)
> {
> - __raw_writel(mac_hi(priv->mac_addr), &slave->regs->sa_hi);
> - __raw_writel(mac_lo(priv->mac_addr), &slave->regs->sa_lo);
> + writel(mac_hi(priv->mac_addr), cpsw_slave_reg(priv, slave, SA_HI));
> + writel(mac_lo(priv->mac_addr), cpsw_slave_reg(priv, slave, SA_LO));
> }
>
> static void _cpsw_adjust_link(struct cpsw_slave *slave,
> @@ -445,8 +488,8 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
> soft_reset(name, &slave->sliver->soft_reset);
>
> /* setup priority mapping */
> - __raw_writel(RX_PRIORITY_MAPPING, &slave->sliver->rx_pri_map);
> - __raw_writel(TX_PRIORITY_MAPPING, &slave->regs->tx_pri_map);
> + writel(RX_PRIORITY_MAPPING, &slave->sliver->rx_pri_map);
> + writel(TX_PRIORITY_MAPPING, cpsw_slave_reg(priv, slave, TX_PRI_MAP));
>
> /* setup max packet size, and mac address */
> __raw_writel(priv->rx_packet_max, &slave->sliver->rx_maxlen);
> @@ -505,7 +548,12 @@ static int cpsw_ndo_open(struct net_device *ndev)
>
> pm_runtime_get_sync(&priv->pdev->dev);
>
> - reg = __raw_readl(&priv->regs->id_ver);
> + reg = readl(&priv->regs->id_ver);
> + priv->cpsw_version = reg;
> + if (reg == CPSW_VERSION_1)
> + priv->slave_reg_ofs = (u32 *)slave_reg_map_ip_v1;
> + else
> + priv->slave_reg_ofs = (u32 *)slave_reg_map_ip_v2;
You didn't provide a way to even use this code, like a dts for a
non-am335x board with the older version.
I think it would be better to start off supporting one version and
have that fully working, and then add the older version, but *really*
add it so that it is actually working.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 2/6] drivers: net: ethernet: davinci_cpdma: add clear api for statistics interrupt
From: Richard Cochran @ 2012-10-18 2:48 UTC (permalink / raw)
To: Mugunthan V N; +Cc: netdev, davem
In-Reply-To: <1350427518-7230-3-git-send-email-mugunthanvnm@ti.com>
On Wed, Oct 17, 2012 at 04:15:14AM +0530, Mugunthan V N wrote:
> Adding api to clear statistics interrupt which is generated by
> CPTS event pend, stat pend, mdio interrupts
As I said, we don't need or want the interrupt for the CPTS. If you
want to add this for the statistics function, go right ahead, but
please make a separate patch or series for that.
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 3/6] drivers: net: ethernet: cpsw: add multicast address to ALE table
From: Richard Cochran @ 2012-10-18 2:49 UTC (permalink / raw)
To: Mugunthan V N; +Cc: netdev, davem
In-Reply-To: <1350427518-7230-4-git-send-email-mugunthanvnm@ti.com>
On Wed, Oct 17, 2012 at 04:15:15AM +0530, Mugunthan V N wrote:
> Adding multicast address to ALE table via netdev ops to subscribe, transmit
> or receive multicast frames to and from the network
Is this somehow related to the time stamping function? If so, how?
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 4/6] ptp: add api to get ptp seq id and event type from skb
From: Richard Cochran @ 2012-10-18 2:55 UTC (permalink / raw)
To: Mugunthan V N; +Cc: netdev, davem
In-Reply-To: <1350427518-7230-5-git-send-email-mugunthanvnm@ti.com>
On Wed, Oct 17, 2012 at 04:15:16AM +0530, Mugunthan V N wrote:
> Cc: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
> include/linux/ptp_classify.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 42 insertions(+), 0 deletions(-)
Sorry, but nak on this one. It is a bit way too long for a static
inline. Also, this driver-specific code, and it doesn't belong here.
Thanks,
Richard
> diff --git a/include/linux/ptp_classify.h b/include/linux/ptp_classify.h
> index 1dc420b..9b3b55b 100644
> --- a/include/linux/ptp_classify.h
> +++ b/include/linux/ptp_classify.h
> @@ -137,4 +137,46 @@ static inline int ptp_filter_init(struct sock_filter *f, int len)
> {OP_RETA, 0, 0, 0 }, /* */ \
> /*L6x*/ {OP_RETK, 0, 0, PTP_CLASS_NONE },
>
> +static inline int ptp_get_skb_event(struct sk_buff *skb, u32 ptp_class,
> + u16 *evt_seqid, u8 *evt_msgtype)
> +{
> + u16 *seqid;
> + unsigned int offset;
> + u8 *msgtype, *data = skb->data;
> +
> + switch (ptp_class) {
> + case PTP_CLASS_V1_IPV4:
> + case PTP_CLASS_V2_IPV4:
> + offset = ETH_HLEN + IPV4_HLEN(data) + UDP_HLEN;
> + break;
> + case PTP_CLASS_V1_IPV6:
> + case PTP_CLASS_V2_IPV6:
> + offset = OFF_PTP6;
> + break;
> + case PTP_CLASS_V2_L2:
> + offset = ETH_HLEN;
> + break;
> + case PTP_CLASS_V2_VLAN:
> + offset = ETH_HLEN + VLAN_HLEN;
> + break;
> + default:
> + return 0;
> + }
> +
> + if (skb->len + ETH_HLEN < offset + OFF_PTP_SEQUENCE_ID + sizeof(*seqid))
> + return 0;
> +
> + if (unlikely(ptp_class & PTP_CLASS_V1))
> + msgtype = data + offset + OFF_PTP_CONTROL;
> + else
> + msgtype = data + offset;
> +
> + seqid = (u16 *)(data + offset + OFF_PTP_SEQUENCE_ID);
> +
> + *evt_seqid = ntohs(*seqid);
> + *evt_msgtype = *msgtype & 0xf;
> +
> + return 0;
> +}
> +
> #endif
> --
> 1.7.0.4
>
^ permalink raw reply
* [PATCH] ipv4: Fix flushing of cached routing informations
From: Steffen Klassert @ 2012-10-18 5:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Currently we can not flush cached pmtu/redirect informations via
the ipv4_sysctl_rtcache_flush sysctl. We need to check the rt_genid
of the old route and reset the nh exeption if the old route is
expired when we bind a new route to a nh exeption.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/route.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 432f4bb..defb98d 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1163,8 +1163,17 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
spin_lock_bh(&fnhe_lock);
if (daddr == fnhe->fnhe_daddr) {
- struct rtable *orig;
+ struct rtable *orig = rcu_dereference(fnhe->fnhe_rth);
+ rcu_assign_pointer(fnhe->fnhe_rth, rt);
+ if (orig) {
+ if (rt_is_expired(orig)) {
+ fnhe->fnhe_gw = 0;
+ fnhe->fnhe_pmtu = 0;
+ fnhe->fnhe_expires = 0;
+ }
+ rt_free(orig);
+ }
if (fnhe->fnhe_pmtu) {
unsigned long expires = fnhe->fnhe_expires;
unsigned long diff = expires - jiffies;
@@ -1181,11 +1190,6 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
} else if (!rt->rt_gateway)
rt->rt_gateway = daddr;
- orig = rcu_dereference(fnhe->fnhe_rth);
- rcu_assign_pointer(fnhe->fnhe_rth, rt);
- if (orig)
- rt_free(orig);
-
fnhe->fnhe_stamp = jiffies;
ret = true;
}
--
1.7.0.4
^ permalink raw reply related
* Re: Bug? TCP shutdown behaviour when deleting local IP addresses
From: Eric Dumazet @ 2012-10-18 6:05 UTC (permalink / raw)
To: Chris Friesen
Cc: netdev, David Miller, Alexey Kuznetsov, James Morris,
Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <507F38D4.102@genband.com>
On Wed, 2012-10-17 at 17:01 -0600, Chris Friesen wrote:
> Hi all,
>
> I sent this to the list yesterday (from another address) but didn't get
> any responses. Accordingly I'm expanding the receiver list to the
> listed maintainers for IPv4/IPv6.
>
> I'm seeing some unexpected (to me, at least) behaviour with local TCP
> connections. The scenario goes as follows:
>
> 1) create new IP address and assign to eth device
> 2) TCP server starts listening on that IP address
> 3) TCP client connects to server
> 4) remove new IP address
> 5) kill server with ctrl-C. At this point it appears that because the
> address was removed the shutdown message isn't processed properly.
> netstat shows the server socket as FIN_WAIT1, but the client socket is
> still ESTABLISHED.
> 6) client writes to the connected socket (this passes with no error)
> 7) client waits for response from server, and waits forever or until
> keepalive expires
>
>
>
> A few points:
>
> This was originally seen on 2.6.27, but I've verified it on 2.6.35. I'll
> see about trying it on current git. I've got really simple
> client/server code if anyone wants to try reproducing.
>
> If we don't remove the address in step 4, then step 5 results in the
> server socket going to FIN_WAIT2 and the client socket going to
> CLOSE_WAIT and step 7 returns right away with zero bytes.
>
> It seems like the waiting forever behaviour in step 7 might be
> legitimate since the address was removed before shutting down the
> server, but it also seems like we should be able to do better given that
> everything is local. In the "remove IP address" case maybe step 6
> should cause some sort of error since the IP address no longer exists?
>
> Incidentally, if we do this sort of scenario with the client and server
> on different hosts then we get a "no route to host" error at step 6.
>
> Curious how this is supposed to work...
>
> Chris
I see no real problem here.
Its like you cut the cable somewhere in the path.
Only timeouts will apply.
And its not keeepalive timeouts in 7) but normal retransmits with
exponential backoff.
Extract of Documentation/networking/ip-sysctl.txt :
tcp_retries1 - INTEGER
This value influences the time, after which TCP decides, that
something is wrong due to unacknowledged RTO retransmissions,
and reports this suspicion to the network layer.
See tcp_retries2 for more details.
RFC 1122 recommends at least 3 retransmissions, which is the
default.
tcp_retries2 - INTEGER
This value influences the timeout of an alive TCP connection,
when RTO retransmissions remain unacknowledged.
Given a value of N, a hypothetical TCP connection following
exponential backoff with an initial RTO of TCP_RTO_MIN would
retransmit N times before killing the connection at the (N+1)th
RTO.
The default value of 15 yields a hypothetical timeout of 924.6
seconds and is a lower bound for the effective timeout.
TCP will effectively time out at the first RTO which exceeds the
hypothetical timeout.
RFC 1122 recommends at least 100 seconds for the timeout,
which corresponds to a value of at least 8.
^ permalink raw reply
* Re: Bug? TCP shutdown behaviour when deleting local IP addresses
From: Chris Friesen @ 2012-10-18 6:33 UTC (permalink / raw)
To: David Miller; +Cc: netdev, kuznet, jmorris, kaber, yoshfuji
In-Reply-To: <20121017.192753.1673804218598802357.davem@davemloft.net>
On 10/17/2012 05:27 PM, David Miller wrote:
> From: Chris Friesen<chris.friesen@genband.com>
> Date: Wed, 17 Oct 2012 17:01:40 -0600
>
>> I sent this to the list yesterday (from another address) but didn't
>> get any responses. Accordingly I'm expanding the receiver list to the
>> listed maintainers for IPv4/IPv6.
>
> It can take more than 24 hours to get a response from people who are
> volunteers, and who will reply to your report because they want to
> rather than because they are obligated.
>
> Posting it again just irritates such people, and will place your
> report at a much lower priority, just FYI.
Thanks for the pointers. I was worried that it had gone unnoticed, but
I guess the level of traffic on netdev is small enough (compared to
something like lkml) that that's generally not an issue.
Chris
^ permalink raw reply
* Re: [PATCH] ipv4: Fix flushing of cached routing informations
From: Julian Anastasov @ 2012-10-18 6:52 UTC (permalink / raw)
To: Steffen Klassert; +Cc: David Miller, netdev
In-Reply-To: <20121018052730.GB27385@secunet.com>
Hello,
On Thu, 18 Oct 2012, Steffen Klassert wrote:
> Currently we can not flush cached pmtu/redirect informations via
> the ipv4_sysctl_rtcache_flush sysctl. We need to check the rt_genid
> of the old route and reset the nh exeption if the old route is
> expired when we bind a new route to a nh exeption.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
> net/ipv4/route.c | 16 ++++++++++------
> 1 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 432f4bb..defb98d 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1163,8 +1163,17 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
> spin_lock_bh(&fnhe_lock);
>
> if (daddr == fnhe->fnhe_daddr) {
> - struct rtable *orig;
> + struct rtable *orig = rcu_dereference(fnhe->fnhe_rth);
> + rcu_assign_pointer(fnhe->fnhe_rth, rt);
May be the above rcu_assign_pointer should be after
... if (!rt->rt_gateway)
rt->rt_gateway = daddr;
part. It is possible another __mkroute_output
caller to get the cached fnhe->fnhe_rth without valid
rt->rt_gateway. rcu_assign_pointer should be after all
writes to rt.
> + if (orig) {
> + if (rt_is_expired(orig)) {
> + fnhe->fnhe_gw = 0;
> + fnhe->fnhe_pmtu = 0;
> + fnhe->fnhe_expires = 0;
> + }
> + rt_free(orig);
> + }
> if (fnhe->fnhe_pmtu) {
> unsigned long expires = fnhe->fnhe_expires;
> unsigned long diff = expires - jiffies;
> @@ -1181,11 +1190,6 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
> } else if (!rt->rt_gateway)
> rt->rt_gateway = daddr;
>
> - orig = rcu_dereference(fnhe->fnhe_rth);
> - rcu_assign_pointer(fnhe->fnhe_rth, rt);
> - if (orig)
> - rt_free(orig);
> -
> fnhe->fnhe_stamp = jiffies;
> ret = true;
> }
> --
> 1.7.0.4
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH] ipv4: Fix flushing of cached routing informations
From: Steffen Klassert @ 2012-10-18 6:54 UTC (permalink / raw)
To: Julian Anastasov; +Cc: David Miller, netdev
In-Reply-To: <alpine.LFD.2.00.1210180927230.1572@ja.ssi.bg>
On Thu, Oct 18, 2012 at 09:52:37AM +0300, Julian Anastasov wrote:
>
> Hello,
>
> On Thu, 18 Oct 2012, Steffen Klassert wrote:
>
> > Currently we can not flush cached pmtu/redirect informations via
> > the ipv4_sysctl_rtcache_flush sysctl. We need to check the rt_genid
> > of the old route and reset the nh exeption if the old route is
> > expired when we bind a new route to a nh exeption.
> >
> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> > ---
> > net/ipv4/route.c | 16 ++++++++++------
> > 1 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index 432f4bb..defb98d 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -1163,8 +1163,17 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
> > spin_lock_bh(&fnhe_lock);
> >
> > if (daddr == fnhe->fnhe_daddr) {
> > - struct rtable *orig;
> > + struct rtable *orig = rcu_dereference(fnhe->fnhe_rth);
> > + rcu_assign_pointer(fnhe->fnhe_rth, rt);
>
> May be the above rcu_assign_pointer should be after
>
> ... if (!rt->rt_gateway)
> rt->rt_gateway = daddr;
>
> part. It is possible another __mkroute_output
> caller to get the cached fnhe->fnhe_rth without valid
> rt->rt_gateway. rcu_assign_pointer should be after all
> writes to rt.
Right, I'll send an updated version.
Thanks!
^ permalink raw reply
* Re: Bug? TCP shutdown behaviour when deleting local IP addresses
From: Chris Friesen @ 2012-10-18 7:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, David Miller, Alexey Kuznetsov, James Morris,
Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <1350540348.26103.1015.camel@edumazet-glaptop>
On 10/18/2012 12:05 AM, Eric Dumazet wrote:
> On Wed, 2012-10-17 at 17:01 -0600, Chris Friesen wrote:
>> I'm seeing some unexpected (to me, at least) behaviour with local TCP
>> connections. The scenario goes as follows:
>>
>> 1) create new IP address and assign to eth device
>> 2) TCP server starts listening on that IP address
>> 3) TCP client connects to server
>> 4) remove new IP address
>> 5) kill server with ctrl-C. At this point it appears that because the
>> address was removed the shutdown message isn't processed properly.
>> netstat shows the server socket as FIN_WAIT1, but the client socket is
>> still ESTABLISHED.
>> 6) client writes to the connected socket (this passes with no error)
>> 7) client waits for response from server, and waits forever or until
>> keepalive expires
>> It seems like the waiting forever behaviour in step 7 might be
>> legitimate since the address was removed before shutting down the
>> server, but it also seems like we should be able to do better given that
>> everything is local. In the "remove IP address" case maybe step 6
>> should cause some sort of error since the IP address no longer exists?
>>
>> Incidentally, if we do this sort of scenario with the client and server
>> on different hosts then we get a "no route to host" error at step 6.
>
> I see no real problem here.
>
> Its like you cut the cable somewhere in the path.
>
> Only timeouts will apply.
While I agree generally, it's a bit unfortunate that we can't (as a
quality of implementation thing) give an earlier notice of failure since
the kernel knows about both ends of the connection even though the IP
address is gone. On the other hand, I imagine that would mean
special-casing things and presumably that would open a whole can of worms.
Thanks for the reply,
Chris
^ permalink raw reply
* [PATCH v2] ipv4: Fix flushing of cached routing informations
From: Steffen Klassert @ 2012-10-18 7:17 UTC (permalink / raw)
To: Julian Anastasov; +Cc: David Miller, netdev
In-Reply-To: <alpine.LFD.2.00.1210180927230.1572@ja.ssi.bg>
Currently we can not flush cached pmtu/redirect informations via
the ipv4_sysctl_rtcache_flush sysctl. We need to check the rt_genid
of the old route and reset the nh exeption if the old route is
expired when we bind a new route to a nh exeption.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/route.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 432f4bb..a8c6512 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1163,8 +1163,12 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
spin_lock_bh(&fnhe_lock);
if (daddr == fnhe->fnhe_daddr) {
- struct rtable *orig;
-
+ struct rtable *orig = rcu_dereference(fnhe->fnhe_rth);
+ if (orig && rt_is_expired(orig)) {
+ fnhe->fnhe_gw = 0;
+ fnhe->fnhe_pmtu = 0;
+ fnhe->fnhe_expires = 0;
+ }
if (fnhe->fnhe_pmtu) {
unsigned long expires = fnhe->fnhe_expires;
unsigned long diff = expires - jiffies;
@@ -1181,7 +1185,6 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
} else if (!rt->rt_gateway)
rt->rt_gateway = daddr;
- orig = rcu_dereference(fnhe->fnhe_rth);
rcu_assign_pointer(fnhe->fnhe_rth, rt);
if (orig)
rt_free(orig);
--
1.7.0.4
^ permalink raw reply related
* Re: Bug? TCP shutdown behaviour when deleting local IP addresses
From: Eric Dumazet @ 2012-10-18 7:29 UTC (permalink / raw)
To: Chris Friesen
Cc: netdev, David Miller, Alexey Kuznetsov, James Morris,
Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <507FAAE1.6000903@genband.com>
On Thu, 2012-10-18 at 01:08 -0600, Chris Friesen wrote:
>
> While I agree generally, it's a bit unfortunate that we can't (as a
> quality of implementation thing) give an earlier notice of failure since
> the kernel knows about both ends of the connection even though the IP
> address is gone. On the other hand, I imagine that would mean
> special-casing things and presumably that would open a whole can of worms.
Really what is the difference between a cable cut and what you are
doing ?
Some frames are lost (Dropped), and sender doesnt 'know' that is
definitive or temporary failure.
If you want faster response, you need to send RST messages, not dropping
frames.
So change your strategy, and add an iptables rule for example ?
^ permalink raw reply
* Re: [PATCH v2] ipv4: Fix flushing of cached routing informations
From: Eric Dumazet @ 2012-10-18 7:40 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Julian Anastasov, David Miller, netdev
In-Reply-To: <20121018071744.GD27385@secunet.com>
On Thu, 2012-10-18 at 09:17 +0200, Steffen Klassert wrote:
> Currently we can not flush cached pmtu/redirect informations via
> the ipv4_sysctl_rtcache_flush sysctl. We need to check the rt_genid
> of the old route and reset the nh exeption if the old route is
> expired when we bind a new route to a nh exeption.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
> net/ipv4/route.c | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 432f4bb..a8c6512 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1163,8 +1163,12 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
> spin_lock_bh(&fnhe_lock);
>
> if (daddr == fnhe->fnhe_daddr) {
> - struct rtable *orig;
> -
> + struct rtable *orig = rcu_dereference(fnhe->fnhe_rth);
> + if (orig && rt_is_expired(orig)) {
> + fnhe->fnhe_gw = 0;
> + fnhe->fnhe_pmtu = 0;
> + fnhe->fnhe_expires = 0;
> + }
> if (fnhe->fnhe_pmtu) {
> unsigned long expires = fnhe->fnhe_expires;
> unsigned long diff = expires - jiffies;
> @@ -1181,7 +1185,6 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
> } else if (!rt->rt_gateway)
> rt->rt_gateway = daddr;
>
> - orig = rcu_dereference(fnhe->fnhe_rth);
> rcu_assign_pointer(fnhe->fnhe_rth, rt);
> if (orig)
> rt_free(orig);
Seems good to me. I'll have to rebase my patch, changing these
rcu_dereference() to rcu_dereference_protected() variant.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: Bug? TCP shutdown behaviour when deleting local IP addresses
From: Mikael Abrahamsson @ 2012-10-18 8:05 UTC (permalink / raw)
To: Chris Friesen
Cc: netdev, David Miller, Alexey Kuznetsov, James Morris,
Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <507F38D4.102@genband.com>
On Wed, 17 Oct 2012, Chris Friesen wrote:
> 1) create new IP address and assign to eth device
> 2) TCP server starts listening on that IP address
> 3) TCP client connects to server
> 4) remove new IP address
I'm a network engineer, as in I work primarily with IP routing. Ever since
I started running Linux back in the mid 90ties I've had a love/hate
relationship with how Linux handles disappearing network connectivity or
IP addresses.
In my mind there are two ways to handle outage:
1. When a network connection (physical interface) goes down, keep
everything as it is, it might come back up again shortly and then we can
continue as if basically nothing happened. TCP was designed for this
considering timeouts can be in hours.
2. When a network connection (physical interface) goes down, wait a few
seconds, give up, reset all connectivity related to that connection,
basically give up.
Now to my question for the netdev people:
Is there functionality in the kernel for a connection manager to easily
accomplish 2, in that when it tries to deconfigure the IP address on the
interface to also kill all TCP connections terminated at that IP? On my
laptop, I regularily have to kill my ssh client after suspend/resume
cycle, because it's been down for quite a while, and the ssh client
doesn't know the TCP connection is now not functional anymore (TCP session
is still up and retransmit won't happen for a while, so the TCP RST from
the server (I use keepalives within SSH) isn't seen for a long time).
Without knowing what's in place right now, I see some behaviours that I'd
like to have:
After resume (or otherwise network connectivity re-established),
connection manager should be able to tell the kernel to:
a) kill all TCP/UDP/other sessions existing which doesn't currently have
an active IP address on the machine. This is for the sake of local
clients.
b) the TCP/SCTP sessions that *do* have an IP, should have their
retransmit timers "reset", so that whatever needs to be sent, should be
sent immediately (or shortly, within a few seconds).
c) tell the kernel to kill all TCP sessions bound to a certain IP, because
the connection manager is going to remove it shortly. Send TCP RSTs or
whatever and close the TCP session, so both ends know that network
connectivity is going down.
This would mean that if I resume my laptop and it establishes network
connectivity, I will then *know* within a few seconds what TCP sessions
are still valid (the ones that aren't will be killed either because
they're bound to an IP that is not available, or a TCP packet is sent out
to which there will be a TCP RST reply from the other side if the TCP
session is closed at that end).
All of this also has implications on IPv6 and autoconfiguration, I don't
know if currently we are closing TCP sessions bound to IPv6 addresses that
are going away due to the RA the addresses were autoconfigured based on,
now doesn't have a valid lifetime anymore and the kernel is going to
remove them. It also applies to both DHCPv6 and DHCPv4 when a lease is
expiring and the IPv4/IPv6 address is going to be removed.
Right now I think the situation with a lot of "hanging" TCP sessions after
connectivity is restored is suboptimal and negatively impacts user
experience. Probably there should be knobs to turn for different user
needs (server or desktop/laptop) because desired behaviour is different in
different use cases.
--
Mikael Abrahamsson email: swmike@swm.pp.se
^ permalink raw reply
* [PATCH v1] net/core: support runtime PM on net_device
From: Ming Lei @ 2012-10-18 8:21 UTC (permalink / raw)
To: David S. Miller, Rafael J. Wysocki
Cc: Oliver Neukum, Alan Stern, netdev, linux-pm, Ming Lei
In ioctl path on net_device, the physical deivce is often
touched, but the physical device may have been put into runtime
suspend state already, so cause some utilitis(ifconfig, ethtool,
...) to return failure in this situation.
This patch enables runtime PM on net_device and mark it as
no_callbacks, and resumes the net_device if physical device
is to be accessed, then suspends it after completion of the
access.
This patch fixes the problem above.
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
v1:
- fix one undefined local variable compile failure
net/core/dev.c | 83 +++++++++++++++++++++++++++++++-------------------
net/core/ethtool.c | 9 ++++--
net/core/net-sysfs.c | 4 +++
3 files changed, 63 insertions(+), 33 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 09cb3f6..d772ce8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -135,6 +135,7 @@
#include <linux/net_tstamp.h>
#include <linux/static_key.h>
#include <net/flow_keys.h>
+#include <linux/pm_runtime.h>
#include "net-sysfs.h"
@@ -4993,39 +4994,24 @@ static int dev_ifsioc_locked(struct net *net, struct ifreq *ifr, unsigned int cm
/*
* Perform the SIOCxIFxxx calls, inside rtnl_lock()
*/
-static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
+static int __dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
{
int err;
struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
const struct net_device_ops *ops;
- if (!dev)
- return -ENODEV;
-
ops = dev->netdev_ops;
switch (cmd) {
case SIOCSIFFLAGS: /* Set interface flags */
return dev_change_flags(dev, ifr->ifr_flags);
- case SIOCSIFMETRIC: /* Set the metric on the interface
- (currently unused) */
- return -EOPNOTSUPP;
-
case SIOCSIFMTU: /* Set the MTU of a device */
return dev_set_mtu(dev, ifr->ifr_mtu);
case SIOCSIFHWADDR:
return dev_set_mac_address(dev, &ifr->ifr_hwaddr);
- case SIOCSIFHWBROADCAST:
- if (ifr->ifr_hwaddr.sa_family != dev->type)
- return -EINVAL;
- memcpy(dev->broadcast, ifr->ifr_hwaddr.sa_data,
- min(sizeof ifr->ifr_hwaddr.sa_data, (size_t) dev->addr_len));
- call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
- return 0;
-
case SIOCSIFMAP:
if (ops->ndo_set_config) {
if (!netif_device_present(dev))
@@ -5049,21 +5035,6 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
if (!netif_device_present(dev))
return -ENODEV;
return dev_mc_del_global(dev, ifr->ifr_hwaddr.sa_data);
-
- case SIOCSIFTXQLEN:
- if (ifr->ifr_qlen < 0)
- return -EINVAL;
- dev->tx_queue_len = ifr->ifr_qlen;
- return 0;
-
- case SIOCSIFNAME:
- ifr->ifr_newname[IFNAMSIZ-1] = '\0';
- return dev_change_name(dev, ifr->ifr_newname);
-
- case SIOCSHWTSTAMP:
- err = net_hwtstamp_validate(ifr);
- if (err)
- return err;
/* fall through */
/*
@@ -5099,6 +5070,56 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
return err;
}
+static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
+{
+ int err;
+ struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
+ const struct net_device_ops *ops;
+
+ if (!dev)
+ return -ENODEV;
+
+ ops = dev->netdev_ops;
+
+ switch (cmd) {
+ case SIOCSIFMETRIC: /* Set the metric on the interface
+ (currently unused) */
+ return -EOPNOTSUPP;
+
+ case SIOCSIFHWBROADCAST:
+ if (ifr->ifr_hwaddr.sa_family != dev->type)
+ return -EINVAL;
+ memcpy(dev->broadcast, ifr->ifr_hwaddr.sa_data,
+ min(sizeof ifr->ifr_hwaddr.sa_data, (size_t) dev->addr_len));
+ call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
+ return 0;
+
+ case SIOCSIFTXQLEN:
+ if (ifr->ifr_qlen < 0)
+ return -EINVAL;
+ dev->tx_queue_len = ifr->ifr_qlen;
+ return 0;
+
+ case SIOCSIFNAME:
+ ifr->ifr_newname[IFNAMSIZ-1] = '\0';
+ return dev_change_name(dev, ifr->ifr_newname);
+
+ case SIOCSHWTSTAMP:
+ err = net_hwtstamp_validate(ifr);
+ if (err)
+ return err;
+ }
+
+ if (pm_runtime_get_sync(&dev->dev) < 0)
+ return -ENODEV;
+
+ err = __dev_ifsioc(net, ifr, cmd);
+
+ pm_runtime_put(&dev->dev);
+
+ return err;
+}
+
/*
* This function handles all "interface"-type I/O control requests. The actual
* 'doing' part of this is dev_ifsioc above.
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 4d64cc2..dae4417 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -25,6 +25,7 @@
#include <linux/slab.h>
#include <linux/rtnetlink.h>
#include <linux/sched.h>
+#include <linux/pm_runtime.h>
/*
* Some useful ethtool_ops methods that're device independent.
@@ -1464,10 +1465,13 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
return -EPERM;
}
+ if (pm_runtime_get_sync(&dev->dev) < 0)
+ return -ENODEV;
+
if (dev->ethtool_ops->begin) {
rc = dev->ethtool_ops->begin(dev);
if (rc < 0)
- return rc;
+ goto exit;
}
old_features = dev->features;
@@ -1648,6 +1652,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
if (old_features != dev->features)
netdev_features_change(dev);
-
+exit:
+ pm_runtime_put(&dev->dev);
return rc;
}
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index bcf02f6..c9adb89 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -23,6 +23,7 @@
#include <linux/export.h>
#include <linux/jiffies.h>
#include <net/wext.h>
+#include <linux/pm_runtime.h>
#include "net-sysfs.h"
@@ -1415,6 +1416,9 @@ int netdev_register_kobject(struct net_device *net)
if (error)
return error;
+ pm_runtime_no_callbacks(dev);
+ pm_runtime_enable(dev);
+
error = register_queue_kobjects(net);
if (error) {
device_del(dev);
--
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