* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Vlad Yasevich @ 2012-07-24 2:27 UTC (permalink / raw)
To: xufeng zhang, sri, davem
Cc: xufengzhang.main, linux-sctp, netdev, linux-kernel
In-Reply-To: <500CDE35.6000009@windriver.com>
xufeng zhang <xufeng.zhang@windriver.com> wrote:
>On 07/19/2012 01:57 PM, xufengzhang.main@gmail.com wrote:
>> When "Invalid Stream Identifier" ERROR happens after process the
>> received DATA chunks, this ERROR chunk is enqueued into outqueue
>> before SACK chunk, so when bundling ERROR chunk with SACK chunk,
>> the ERROR chunk is always placed first in the packet because of
>> the chunk's position in the outqueue.
>> This violates sctp specification:
>> RFC 4960 6.5. Stream Identifier and Stream Sequence Number
>> ...The endpoint may bundle the ERROR chunk in the same
>> packet as the SACK as long as the ERROR follows the SACK.
>> So we must place SACK first when bundling "Invalid Stream Identifier"
>> ERROR and SACK in one packet.
>> Although we can do that by enqueue SACK chunk into outqueue before
>> ERROR chunk, it will violate the side-effect interpreter processing.
>> It's easy to do this job when dequeue chunks from the outqueue,
>> by this way, we introduce a flag 'has_isi_err' which indicate
>> whether or not the "Invalid Stream Identifier" ERROR happens.
>>
>> Signed-off-by: Xufeng Zhang<xufeng.zhang@windriver.com>
>> ---
>> include/net/sctp/structs.h | 2 ++
>> net/sctp/output.c | 26 ++++++++++++++++++++++++++
>> 2 files changed, 28 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
>> index 88949a9..5adf4de 100644
>> --- a/include/net/sctp/structs.h
>> +++ b/include/net/sctp/structs.h
>> @@ -842,6 +842,8 @@ struct sctp_packet {
>> has_sack:1, /* This packet contains a SACK chunk. */
>> has_auth:1, /* This packet contains an AUTH chunk */
>> has_data:1, /* This packet contains at least 1 DATA chunk */
>> + has_isi_err:1, /* This packet contains a "Invalid Stream
>> + * Identifier" ERROR chunk */
>> ipfragok:1, /* So let ip fragment this packet */
>> malloced:1; /* Is it malloced? */
>> };
>> diff --git a/net/sctp/output.c b/net/sctp/output.c
>> index 817174e..77fb1ae 100644
>> --- a/net/sctp/output.c
>> +++ b/net/sctp/output.c
>> @@ -79,6 +79,7 @@ static void sctp_packet_reset(struct sctp_packet
>*packet)
>> packet->has_sack = 0;
>> packet->has_data = 0;
>> packet->has_auth = 0;
>> + packet->has_isi_err = 0;
>> packet->ipfragok = 0;
>> packet->auth = NULL;
>> }
>> @@ -267,6 +268,7 @@ static sctp_xmit_t sctp_packet_bundle_sack(struct
>sctp_packet *pkt,
>> sctp_xmit_t sctp_packet_append_chunk(struct sctp_packet *packet,
>> struct sctp_chunk *chunk)
>> {
>> + struct sctp_chunk *lchunk;
>> sctp_xmit_t retval = SCTP_XMIT_OK;
>> __u16 chunk_len = WORD_ROUND(ntohs(chunk->chunk_hdr->length));
>>
>> @@ -316,7 +318,31 @@ sctp_xmit_t sctp_packet_append_chunk(struct
>sctp_packet *packet,
>> packet->has_cookie_echo = 1;
>> break;
>>
>> + case SCTP_CID_ERROR:
>> + if (chunk->subh.err_hdr->cause& SCTP_ERROR_INV_STRM)
>> + packet->has_isi_err = 1;
>> + break;
>> +
>> case SCTP_CID_SACK:
>> + /* RFC 4960
>> + * 6.5 Stream Identifier and Stream Sequence Number
>> + * The endpoint may bundle the ERROR chunk in the same
>> + * packet as the SACK as long as the ERROR follows the SACK.
>> + */
>> + if (packet->has_isi_err) {
>> + if (list_is_singular(&packet->chunk_list))
>> + list_add(&chunk->list,&packet->chunk_list);
>> + else {
>> + lchunk = list_first_entry(&packet->chunk_list,
>> + struct sctp_chunk, list);
>> + list_add(&chunk->list,&lchunk->list);
>> + }
>>
>And I should clarify the above judgment code.
>AFAIK, there should be two cases for the bundling when invalid stream
>identifier error happens:
>1). COOKIE_ACK ERROR SACK
>2). ERROR SACK
>So I need to deal with the two cases differently.
>
Sorry but I just don't buy that the above are the only 2 cases. What if there are addip chunks as well? What if there are some other extensions also. This code has to be generic enough to handle any condition.
- vlad
>
>Thanks,
>Xufeng Zhang
>> + packet->size += chunk_len;
>> + chunk->transport = packet->transport;
>> + packet->has_sack = 1;
>> + goto finish;
>> + }
>> +
>> packet->has_sack = 1;
>> break;
>>
>>
--
Sent from my Android phone with SkitMail. Please excuse my brevity.
^ permalink raw reply
* Re: [RFC] net:phy:phylib: phy shares the same interrupt with mac
From: Jason Lin @ 2012-07-24 1:58 UTC (permalink / raw)
To: Andy Fleming; +Cc: Florian Fainelli, netdev
In-Reply-To: <CAGCDX1=Y_UWpmo0RiexYh-MJ8v6wwXZ=14h+NS5cmi5Nboytow@mail.gmail.com>
Any comments will be appreciated.
Thanks.
2012/7/13 Jason Lin <kernel.jason@gmail.com>:
> Dear:
> But I think there have some performance issues.
> Because the phy should execute its own ISR only when the interrupt is
> issued by phy.
> This will cause some unnecessary MDIO access.
>
> By the way, PHY_DUMMY_IRQ is defined in the platform definition file.
> One should get PHY_DUMMY_IRQ by platform_get_irq function.
> So the driver will look like this:
>
> /*** platform related file ***************/
> static struct resource mac_0_resources[] = {
> {
> .start = MAC_PA_BASE,
> .end = MAC_PA_BASE + SZ_4K - 1,
> .flags = IORESOURCE_MEM,
> }, {
> .start = IRQ_MAC,
> .flags = IORESOURCE_IRQ,
> }, {
> .start = IRQ_PHY,
> .flags = IORESOURCE_IRQ,
> },
> };
>
>
> /*** driver file ***************/
> mydriver_probe(struct platform_device *pdev)
> {
> int mac_irq, phy_irq, i;
>
> mac_irq = platform_get_irq(pdev, 0);
> if (mac_irq < 0)
> return mac_irq;
>
> phy_irq = platform_get_irq(pdev, 1);
> if (phy_irq < 0)
> phy_irq = PHY_POLL;
>
> for (i = 0; i < PHY_MAX_ADDR; i++)
> priv->mii_bus->irq[i] = phy_irq;
> }
>
> mydriver_napi_poll(struct napi_struct *napi, int budget)
> {
> struct mydriver_priv *priv = = container_of(napi, struct
> mydriver_priv, napi);
> struct phy_device *phydev = priv->phydev;
> unsigned int status = get_int_status();
>
> #ifdef CONFIG_MAC_CONTROL_PHY_INT
> if (status & PHY_INT) {
> tasklet_schedule(&phydev->phy_task);
> }
> #endif
>
> }
>
> /**** phy lib modification ***************/
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 3cbda08..482f7e5 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -416,7 +416,7 @@ out_unlock:
> }
> EXPORT_SYMBOL(phy_start_aneg);
>
> -
> +static void phy_tasklet(unsigned long data);
> static void phy_change(struct work_struct *work);
>
> /**
> @@ -523,10 +523,12 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
> * context, so we need to disable the irq here. A work
> * queue will write the PHY to disable and clear the
> * interrupt, and then reenable the irq line. */
> - disable_irq_nosync(irq);
> +/* disable_irq_nosync(irq);
> atomic_inc(&phydev->irq_disable);
>
> - schedule_work(&phydev->phy_queue);
> + schedule_work(&phydev->phy_queue);*/
> +
> + tasklet_schedule(&phydev->phy_task);
>
> return IRQ_HANDLED;
> }
> @@ -591,6 +593,7 @@ int phy_start_interrupts(struct phy_device *phydev)
> {
> int err = 0;
>
> + tasklet_init(&phydev->phy_task, phy_tasklet, (unsigned long)phydev);
> INIT_WORK(&phydev->phy_queue, phy_change);
>
> atomic_set(&phydev->irq_disable, 0);
> @@ -633,6 +636,7 @@ int phy_stop_interrupts(struct phy_device *phydev)
> * possibly pending and take care of the matter below.
> */
> cancel_work_sync(&phydev->phy_queue);
> + tasklet_kill(&phydev->phy_task);
> /*
> * If work indeed has been cancelled, disable_irq() will have
> * been left unbalanced from phy_interrupt() and enable_irq()
> @@ -645,7 +649,13 @@ int phy_stop_interrupts(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(phy_stop_interrupts);
>
>
> -
> +static void phy_tasklet(unsigned long data)
> +{
> + struct phy_device *phydev = (struct phy_device *)data;
> + disable_irq_nosync(phydev->irq);
> + atomic_inc(&phydev->irq_disable);
> + schedule_work(&phydev->phy_queue);
> +}
> /**
> * phy_change - Scheduled by the phy_interrupt/timer to handle PHY changes
> * @work: work_struct that describes the work to be done
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index c599f7e..04203a7 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -330,6 +330,7 @@ struct phy_device {
> /* Interrupt and Polling infrastructure */
> struct work_struct phy_queue;
> struct delayed_work state_queue;
> + struct tasklet_struct phy_task;
> atomic_t irq_disable;
>
> struct mutex lock;
>
> /*** end modification **********************************/
>
> how about this modification?
> F.Y.I.
> Thanks.
>
> 2012/7/13 Jason Lin <kernel.jason@gmail.com>:
>> Thank for your reply.
>> My case is the second one.
>> change request_irq with IRQF_SHARED, and assign the same MAC irq number to phy
>> (mii_bus->irq[i] = mac_irq)
>> This way can fix my issue.
>>
>> Thanks again.
>>
>> 2012/7/13 Andy Fleming <afleming@gmail.com>:
>>> On Tue, Jul 10, 2012 at 8:32 PM, Jason Lin <kernel.jason@gmail.com> wrote:
>>>> Dear :
>>>> I describe my situation again.
>>>> In my hardware design, the interrupt (INT) pin of phy is connected to
>>>> the INT input pin of MAC.
>>>> In other words, one of triggering interrupt condition of MAC is
>>>> related to phy's interrupt.
>>>> So the phy will share the same "IRQ pin" with MAC.
>>>> As described above, the best solution is the INT pin of phy is
>>>> connected to architecture independently.
>>>> But, the hardware architecture cannot modify easily.
>>>> So I think
>>>> 1. We can assign dummy IRQ number (which is a empty IRQ number but
>>>> large than zero) to phy.
>>>> phydev->irq = PHY_DUMMY_IRQ (this value is depend on architecture)
>>>> 2. Change to do the soft IRQ in phy's ISR.
>>>> To schedule workqueue in this soft IRQ.
>>>> In this way, the MAC can schedule phy's soft IRQ to do phy's work
>>>> queue (to do ack phy's interrupt ... etc).
>>>>
>>>> Dose it make sense?
>>>>
>>>
>>> If PHY_IGNORE_INTERRUPT doesn't currently work to allow a MAC's driver
>>> to manage PHY interrupts, then we need to fix PHY Lib to support this
>>> correctly. PHY_IGNORE_INTERRUPT was meant for this purpose. We don't
>>> want to start defining interrupt numbers which may conflict with real
>>> numbers, and have to be constantly re-defined by various systems to
>>> avoid that.
>>>
>>> Your notion of making the phy_enable_interrupts() and
>>> phy_disable_interrupts() code available to the MAC drivers sounds like
>>> the right approach to me. But you might want to implement your own
>>> version. The biggest problem with PHY interrupts is that masking them
>>> requires an MDIO transaction, which is *slow*. If you can mask the
>>> interrupt by writing a bit in a memory-mapped register, it's far
>>> better to do it that way, at interrupt time, and *then* schedule the
>>> PHY code to be run from a work queue. However, your driver probably
>>> already *does* have a workqueue of some sort. If so, what you should
>>> probably do is implement a new version of phy_change(), that doesn't
>>> have to deal with the weird PHY interrupt masking issues. Something
>>> like this:
>>>
>>> mydriver_phy_change(struct mydriver_priv *priv)
>>> {
>>> int err;
>>> struct phy_device *phydev = priv->phydev;
>>>
>>> if (phydev->drv->did_interrupt &&
>>> !phydev->drv->did_interrupt(phydev))
>>> goto ignore;
>>>
>>> err = phy_disable_interrupts(phydev);
>>>
>>> if (err)
>>> goto phy_err;
>>>
>>> mutex_lock(&phydev->lock);
>>> if ((PHY_RUNNING == phydev->state) || (PHY_NOLINK == phydev->state))
>>> phydev->state = PHY_CHANGELINK;
>>> mutex_unlock(&phydev->lock);
>>>
>>> /* Reenable interrupts */
>>> if (PHY_HALTED != phydev->state)
>>> err = phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED);
>>>
>>> if (err)
>>> goto irq_enable_err;
>>>
>>> /* reschedule state queue work to run as soon as possible */
>>> cancel_delayed_work_sync(&phydev->state_queue);
>>> schedule_delayed_work(&phydev->state_queue, 0);
>>>
>>> return;
>>>
>>> ignore:
>>> return;
>>>
>>> irq_enable_err:
>>> phy_err:
>>> phy_error(phydev);
>>> }
>>>
>>>
>>> Of course, now I re-read your question, and I'm not sure I've
>>> interpreted it correctly. Are you saying your MAC has control of the
>>> PHY interrupt (ie - the interrupt sets a bit in some event register in
>>> your MAC, and you can MASK/ACK it from your driver), or that the PHY
>>> shares the same interrupt pin?
>>>
>>> If it's the second one, you don't need to do anything, but allow your
>>> MAC driver to register its interrupt as a shared interrupt, and allow
>>> the PHY to manage its own interrupts, as usual.
>>>
>>> Andy
^ permalink raw reply
* Re: [PATCH] ppp: add 64 bit stats
From: Kevin Groeneveld @ 2012-07-24 1:53 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1343059184.2626.11027.camel@edumazet-glaptop>
On Mon, Jul 23, 2012 at 11:59 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> Would proper synchronization in this case just be wrapping the updates
>> in a spin_lock/spin_unlock?
>
> Would be fine (if the proper BH safe variant is used), or you could also
> use atomic64_t.
Which would you recommend, spin locks or atomic64_t?
atomic64_t seems like it would be simpler.
^ permalink raw reply
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Xufeng Zhang @ 2012-07-24 1:53 UTC (permalink / raw)
To: Neil Horman
Cc: xufeng zhang, vyasevich, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <20120723121411.GC16518@hmsreliant.think-freely.org>
On 7/23/12, Neil Horman <nhorman@tuxdriver.com> 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.
(Please ignore the duplicate messages if you received, sorry for this!)
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.
Thanks,
Xufeng Zhang
>
> Regards
> Neil
>
>
^ permalink raw reply
* Re: [PATCH RESEND net/for-next V1 1/1] IB/ipoib: break linkage to neighbouring system
From: David Miller @ 2012-07-24 1:34 UTC (permalink / raw)
To: ogerlitz-VPRAkNaXOzVWk0Htik3J/w
Cc: roland-DgEjT+Ai2ygdnm+yROfE0A, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
erezsh-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
shlomop-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <1342714502-11301-2-git-send-email-ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
From: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Date: Thu, 19 Jul 2012 19:15:02 +0300
> -static void neigh_add_path(struct sk_buff *skb, struct neighbour *n, struct net_device *dev)
> +static void neigh_add_path(struct sk_buff *skb, u8 *daddr,
> + struct net_device *dev)
Please line up "struct net_device *dev" with the first column after the
openning parenthesis on the first line.
> + return jhash(daddr+12, 8, 0xFFFFFF & *(u32 *) daddr) & htbl->mask;
It would be tons faster to go:
u32 *daddr_32 = (u32 *) daddr;
hv = jhash_3words(daddr_32[3], daddr_32[4], 0xFFFFFF & daddr_32[0], 0);
hv &= htbl->mask;
Plain jhash() has loops and various conditions that are entirely
unnecessary when you know how much you are hashing ahead of time.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] r8169: revert "add byte queue limit support".
From: David Miller @ 2012-07-24 1:27 UTC (permalink / raw)
To: romieu; +Cc: hayeswang, netdev, a_villacis, jwboyer
In-Reply-To: <20120723205555.GA4392@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Mon, 23 Jul 2012 22:55:55 +0200
> This reverts commit 036dafa28da1e2565a8529de2ae663c37b7a0060.
>
> First it appears in bisection, then reverting it solves the usual
> netdev watchdog problem for different people. I don't have a proper
> fix yet so get rid of it.
>
> Bisected-and-reported-by: Alex Villacís Lasso <a_villacis@palosanto.com>
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Cc: Josh Boyer <jwboyer@redhat.com>
> Cc: Hayes Wang <hayeswang@realtek.com>
> ---
>
> The original 036da... commit has been modified due to the newly introduced
> skb_tx_timestamp in rtl8169_start_xmit. The herein included patch qualifies
> for 3.4-stable as well.
Applied to net-next and queued up for -stable, thanks.
^ permalink raw reply
* Re: [GIT PULL nf-next] IPVS
From: David Miller @ 2012-07-24 1:11 UTC (permalink / raw)
To: horms; +Cc: pablo, lvs-devel, netdev, netfilter-devel, wensong, ja
In-Reply-To: <20120724010105.GH22631@verge.net.au>
From: Simon Horman <horms@verge.net.au>
Date: Tue, 24 Jul 2012 10:01:05 +0900
> On Mon, Jul 23, 2012 at 04:35:08PM -0700, David Miller wrote:
>> From: Simon Horman <horms@verge.net.au>
>> Date: Tue, 24 Jul 2012 08:28:55 +0900
>>
>> > please consider the following enhancements to IPVS for inclusion in 3.6.
>>
>> The merge window has just openned, therefore any new work should have
>> been submitted and queued up already.
>
> Sorry for my lateness. I can hold these changes for 3.7 if that is the
> best option at this point.
No worries, there is always the next merge window :-)
^ permalink raw reply
* Re: [GIT PULL nf-next] IPVS
From: Simon Horman @ 2012-07-24 1:01 UTC (permalink / raw)
To: David Miller; +Cc: pablo, lvs-devel, netdev, netfilter-devel, wensong, ja
In-Reply-To: <20120723.163508.1145167951335987230.davem@davemloft.net>
On Mon, Jul 23, 2012 at 04:35:08PM -0700, David Miller wrote:
> From: Simon Horman <horms@verge.net.au>
> Date: Tue, 24 Jul 2012 08:28:55 +0900
>
> > please consider the following enhancements to IPVS for inclusion in 3.6.
>
> The merge window has just openned, therefore any new work should have
> been submitted and queued up already.
Sorry for my lateness. I can hold these changes for 3.7 if that is the
best option at this point.
^ permalink raw reply
* Re: [PATCH 2/2] ipv4: Change rt->rt_iif encoding.
From: David Miller @ 2012-07-24 0:43 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.1207240226340.2633@ja.ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Tue, 24 Jul 2012 03:24:38 +0300 (EEST)
> On Mon, 23 Jul 2012, David Miller wrote:
>
>> Hmmm, the problem is that when we decapsulate VLAN devices, we're left
>> with the parent device's index in skb->skb_iif.
>
> Not sure if it is a problem with VLANs, can be also
> with some virtual devices but may be they use dev_forward_skb()
> where skb_iif is zeroed.
dev_forward_skb() gives the packet to netif_rx() which will thus send
the packet down to __netif_receive_skb() which will set the
skb->skb_iif to the new device's ifindex. It will not stay at zero
:-)
> I was also worrying about ip_mc_output looping packets with
> skb_iif because skb_clone copies the field but may be such loops
> happen only for locally originated traffic where skb_iif starts with
> 0.
Loopback of multicast packets is done in ip_mc_output(), via
the clone that you mention, via dev_loopback_xmit().
dev_loopback_xmit() gives the packet to netif_rx_ni() which again
lands it back at __netif_receive_skb(), which (with my changes)
will adjust the skb->skb_iif to match whatever sits in skb->dev
at the time.
It should be alright.
^ permalink raw reply
* Re: [PATCH 2/2] ipv4: Change rt->rt_iif encoding.
From: Julian Anastasov @ 2012-07-24 0:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20120723.161446.36265037346365173.davem@davemloft.net>
Hello,
On Mon, 23 Jul 2012, David Miller wrote:
> Hmmm, the problem is that when we decapsulate VLAN devices, we're left
> with the parent device's index in skb->skb_iif.
Not sure if it is a problem with VLANs, can be also
with some virtual devices but may be they use dev_forward_skb()
where skb_iif is zeroed.
> That's what all of that "orig_dev" logic in __netif_receive_skb() is
> all about. The skb->dev is rewritten by vlan_do_receive() for that
> case.
This is the part that I didn't know for skb_iif. I was
also worrying about ip_mc_output looping packets with skb_iif
because skb_clone copies the field but may be such loops
happen only for locally originated traffic where skb_iif starts
with 0.
Still, the comment in ipmr_queue_xmit() and the
IPSKB_FORWARDED bit puzzles me:
* RFC1584 teaches, that DVMRP/PIM router must deliver packets locally
* not only before forwarding, but after forwarding on all output
* interfaces. It is clear, if mrouter runs a multicasting
ip_mr_forward preserves one copy to local app,
calls ipmr_queue_xmit() where packet can be looped?
ip_mr_input():
/* Packet is looped back after forward, it should not be
* forwarded second time, but still can be delivered locally.
*/
I really hope the case is for locally originated
multicast, not for packets received from network.
> I wonder if we should just get rid of all of that orig_dev logic and
> simply update skb->skb_iif every time we hit the code starting at
> label "another_round"
I don't have opinion about this problem. I see your
new change for __netif_receive_skb, the skb_iif places are
something that I have to check for me...
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [BUG] tg3: frequent freezes
From: Michael Chan @ 2012-07-24 0:13 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1342508189.2168.14.camel@LTIRV-MCHAN1.corp.ad.broadcom.com>
On Mon, 2012-07-16 at 23:56 -0700, Michael Chan wrote:
> On Tue, 2012-07-17 at 07:35 +0200, Eric Dumazet wrote:
> > [ 266.248330] tg3 0000:09:00.0: eth0: 0x00004800: 0x080303fe,
> > 0x00000010, 0x00000000, 0x00000020
>
> RDMAC_STATUS (0x4804) is showing 0x10 which is parity error.
>
> >[ 266.247878] tg3 0000:09:00.0: eth0: 0x00000000: 0x167314e4,
> >0x40100506, 0x02000002, 0x00000010
>
> PCI Status (0x06) is also showing SERR (0x4000)
>
> Assuming your chip is not a bad chip, I'll have to look around for a
> 5755M and see if I can duplicate the problem.
Hi Eric, I found the same 5755m NIC as yours but I couldn't duplicate
any problems. I ran netperf in a loop and it was running fine.
Here's my device and it should be identical to yours:
tg3.c:v3.123 (March 21, 2012)
tg3 0000:20:00.0: eth0: Tigon3 [partno(BCM95755m) rev a002] (PCI Express) MAC address 00:10:18:00:00:00
tg3 0000:20:00.0: eth0: attached PHY is 5755 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
tg3 0000:20:00.0: eth0: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
tg3 0000:20:00.0: eth0: dma_rwctrl[76180000] dma_mask[64-bit]
^ permalink raw reply
* [PATCH 3/3] ipv4: Change rt->rt_iif encoding.
From: David Miller @ 2012-07-23 23:45 UTC (permalink / raw)
To: netdev; +Cc: ja
On input packet processing, rt->rt_iif will be zero if we should
use skb->dev->ifindex.
Since we access rt->rt_iif consistently via inet_iif(), that is
the only spot whose interpretation have to adjust.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/route.h | 6 +++++-
net/ipv4/route.c | 8 ++++----
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/net/route.h b/include/net/route.h
index 60d611d..c29ef27 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -277,7 +277,11 @@ static inline struct rtable *ip_route_newports(struct flowi4 *fl4, struct rtable
static inline int inet_iif(const struct sk_buff *skb)
{
- return skb_rtable(skb)->rt_iif;
+ int iif = skb_rtable(skb)->rt_iif;
+
+ if (iif)
+ return iif;
+ return skb->skb_iif;
}
extern int sysctl_ip_default_ttl;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index f6be781..6bcb8fc 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1309,7 +1309,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
rth->rt_flags = RTCF_MULTICAST;
rth->rt_type = RTN_MULTICAST;
rth->rt_is_input= 1;
- rth->rt_iif = dev->ifindex;
+ rth->rt_iif = 0;
rth->rt_pmtu = 0;
rth->rt_gateway = 0;
if (our) {
@@ -1435,7 +1435,7 @@ static int __mkroute_input(struct sk_buff *skb,
rth->rt_flags = flags;
rth->rt_type = res->type;
rth->rt_is_input = 1;
- rth->rt_iif = in_dev->dev->ifindex;
+ rth->rt_iif = 0;
rth->rt_pmtu = 0;
rth->rt_gateway = 0;
@@ -1608,7 +1608,7 @@ local_input:
rth->rt_flags = flags|RTCF_LOCAL;
rth->rt_type = res.type;
rth->rt_is_input = 1;
- rth->rt_iif = dev->ifindex;
+ rth->rt_iif = 0;
rth->rt_pmtu = 0;
rth->rt_gateway = 0;
if (res.type == RTN_UNREACHABLE) {
@@ -1772,7 +1772,7 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
rth->rt_flags = flags;
rth->rt_type = type;
rth->rt_is_input = 0;
- rth->rt_iif = orig_oif ? : dev_out->ifindex;
+ rth->rt_iif = orig_oif ? : 0;
rth->rt_pmtu = 0;
rth->rt_gateway = 0;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/3] net: Make skb->skb_iif always track skb->dev
From: David Miller @ 2012-07-23 23:45 UTC (permalink / raw)
To: netdev; +Cc: ja
Make it follow device decapsulation, from things such as VLAN and
bonding.
The stuff that actually cares about pre-demuxed device pointers, is
handled by the "orig_dev" variable in __netif_receive_skb(). And
the only consumer of that is the po->origdev feature of AF_PACKET
sockets.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/core/dev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index cca02ae..0ebaea1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3173,8 +3173,6 @@ static int __netif_receive_skb(struct sk_buff *skb)
if (netpoll_receive_skb(skb))
return NET_RX_DROP;
- if (!skb->skb_iif)
- skb->skb_iif = skb->dev->ifindex;
orig_dev = skb->dev;
skb_reset_network_header(skb);
@@ -3186,6 +3184,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
rcu_read_lock();
another_round:
+ skb->skb_iif = skb->dev->ifindex;
__this_cpu_inc(softnet_data.processed);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/3] ipv4: Prepare for change of rt->rt_iif encoding.
From: David Miller @ 2012-07-23 23:45 UTC (permalink / raw)
To: netdev; +Cc: ja
Use inet_iif() consistently, and for TCP record the input interface of
cached RX dst in inet sock.
rt->rt_iif is going to be encoded differently, so that we can
legitimately cache input routes in the FIB info more aggressively.
When the input interface is "use SKB device index" the rt->rt_iif will
be set to zero.
This forces us to move the TCP RX dst cache installation into the ipv4
specific code, and as well it should since doing the route caching for
ipv6 is pointless at the moment since it is not inspected in the ipv6
input paths yet.
Also, remove the unlikely on dst->obsolete, all ipv4 dsts have
obsolete set to a non-zero value to force invocation of the check
callback.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/inet_sock.h | 1 +
net/dccp/ipv4.c | 2 +-
net/ipv4/icmp.c | 2 +-
net/ipv4/ip_sockglue.c | 5 ++---
net/ipv4/route.c | 2 +-
net/ipv4/tcp_input.c | 12 ------------
net/ipv4/tcp_ipv4.c | 24 ++++++++++++++++++------
net/sched/cls_route.c | 2 +-
net/sched/em_meta.c | 2 +-
net/sctp/protocol.c | 2 +-
10 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 924d7b9..613cfa4 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -172,6 +172,7 @@ struct inet_sock {
int uc_index;
int mc_index;
__be32 mc_addr;
+ int rx_dst_ifindex;
struct ip_mc_socklist __rcu *mc_list;
struct inet_cork_full cork;
};
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 25428d0..176ecdb 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -481,7 +481,7 @@ static struct dst_entry* dccp_v4_route_skb(struct net *net, struct sock *sk,
struct rtable *rt;
const struct iphdr *iph = ip_hdr(skb);
struct flowi4 fl4 = {
- .flowi4_oif = skb_rtable(skb)->rt_iif,
+ .flowi4_oif = inet_iif(skb),
.daddr = iph->saddr,
.saddr = iph->daddr,
.flowi4_tos = RT_CONN_FLAGS(sk),
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index f2a06be..f2eccd5 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -571,7 +571,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
rcu_read_lock();
if (rt_is_input_route(rt) &&
net->ipv4.sysctl_icmp_errors_use_inbound_ifaddr)
- dev = dev_get_by_index_rcu(net, rt->rt_iif);
+ dev = dev_get_by_index_rcu(net, inet_iif(skb_in));
if (dev)
saddr = inet_select_addr(dev, 0, RT_SCOPE_LINK);
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index de29f46..5eea4a8 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1027,10 +1027,9 @@ e_inval:
void ipv4_pktinfo_prepare(struct sk_buff *skb)
{
struct in_pktinfo *pktinfo = PKTINFO_SKB_CB(skb);
- const struct rtable *rt = skb_rtable(skb);
- if (rt) {
- pktinfo->ipi_ifindex = rt->rt_iif;
+ if (skb_rtable(skb)) {
+ pktinfo->ipi_ifindex = inet_iif(skb);
pktinfo->ipi_spec_dst.s_addr = fib_compute_spec_dst(skb);
} else {
pktinfo->ipi_ifindex = 0;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 34017be..f6be781 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -848,7 +848,7 @@ void ip_rt_send_redirect(struct sk_buff *skb)
if (log_martians &&
peer->rate_tokens == ip_rt_redirect_number)
net_warn_ratelimited("host %pI4/if%d ignores redirects for %pI4 to %pI4\n",
- &ip_hdr(skb)->saddr, rt->rt_iif,
+ &ip_hdr(skb)->saddr, inet_iif(skb),
&ip_hdr(skb)->daddr, &rt->rt_gateway);
#endif
}
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 21d7f8f..3e07a64 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5391,18 +5391,6 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
{
struct tcp_sock *tp = tcp_sk(sk);
- if (sk->sk_rx_dst) {
- struct dst_entry *dst = sk->sk_rx_dst;
- if (unlikely(dst->obsolete)) {
- if (dst->ops->check(dst, 0) == NULL) {
- dst_release(dst);
- sk->sk_rx_dst = NULL;
- }
- }
- }
- if (unlikely(sk->sk_rx_dst == NULL))
- sk->sk_rx_dst = dst_clone(skb_dst(skb));
-
/*
* Header prediction.
* The code loosely follows the one in the famous
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index bc5432e..3e30548 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1618,6 +1618,20 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
sock_rps_save_rxhash(sk, skb);
+ if (sk->sk_rx_dst) {
+ struct dst_entry *dst = sk->sk_rx_dst;
+ if (dst->ops->check(dst, 0) == NULL) {
+ dst_release(dst);
+ sk->sk_rx_dst = NULL;
+ }
+ }
+ if (unlikely(sk->sk_rx_dst == NULL)) {
+ struct inet_sock *icsk = inet_sk(sk);
+ struct rtable *rt = skb_rtable(skb);
+
+ sk->sk_rx_dst = dst_clone(&rt->dst);
+ icsk->rx_dst_ifindex = inet_iif(skb);
+ }
if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len)) {
rsk = sk;
goto reset;
@@ -1700,14 +1714,12 @@ void tcp_v4_early_demux(struct sk_buff *skb)
skb->destructor = sock_edemux;
if (sk->sk_state != TCP_TIME_WAIT) {
struct dst_entry *dst = sk->sk_rx_dst;
+ struct inet_sock *icsk = inet_sk(sk);
if (dst)
dst = dst_check(dst, 0);
- if (dst) {
- struct rtable *rt = (struct rtable *) dst;
-
- if (rt->rt_iif == dev->ifindex)
- skb_dst_set_noref(skb, dst);
- }
+ if (dst &&
+ icsk->rx_dst_ifindex == dev->ifindex)
+ skb_dst_set_noref(skb, dst);
}
}
}
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 36fec42..44f405c 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -143,7 +143,7 @@ static int route4_classify(struct sk_buff *skb, const struct tcf_proto *tp,
if (head == NULL)
goto old_method;
- iif = ((struct rtable *)dst)->rt_iif;
+ iif = inet_iif(skb);
h = route4_fastmap_hash(id, iif);
if (id == head->fastmap[h].id &&
diff --git a/net/sched/em_meta.c b/net/sched/em_meta.c
index 4790c69..4ab6e33 100644
--- a/net/sched/em_meta.c
+++ b/net/sched/em_meta.c
@@ -264,7 +264,7 @@ META_COLLECTOR(int_rtiif)
if (unlikely(skb_rtable(skb) == NULL))
*err = -1;
else
- dst->value = skb_rtable(skb)->rt_iif;
+ dst->value = inet_iif(skb);
}
/**************************************************************************
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 9c90811..1f89c4e 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -568,7 +568,7 @@ static void sctp_v4_get_saddr(struct sctp_sock *sk,
/* What interface did this skb arrive on? */
static int sctp_v4_skb_iif(const struct sk_buff *skb)
{
- return skb_rtable(skb)->rt_iif;
+ return inet_iif(skb);
}
/* Was this packet marked by Explicit Congestion Notification? */
--
1.7.10.4
^ permalink raw reply related
* Re: [3.5 regression / mcs7830 / bisected] bridge constantly toggeling between disabled and forwarding
From: Michael Leun @ 2012-07-23 23:36 UTC (permalink / raw)
To: linux; +Cc: davem, netdev, linux-kernel, gregkh
In-Reply-To: <20120723091504.2d035d28@xenia.leun.net>
On Mon, 23 Jul 2012 09:15:04 +0200
Michael Leun <lkml20120218@newton.leun.net> wrote:
[see issue description below]
Bisecting yielded
b1ff4f96fd1c63890d78d8939c6e0f2b44ce3113 is the first bad commit
commit b1ff4f96fd1c63890d78d8939c6e0f2b44ce3113
Author: Ondrej Zary <linux@rainbow-software.org>
Date: Fri Jun 1 10:29:08 2012 +0000
mcs7830: Implement link state detection
Add .status callback that detects link state changes.
Tested with MCS7832CV-AA chip (9710:7830, identified as rev.C by the driver).
Fixes https://bugzilla.kernel.org/show_bug.cgi?id=28532
Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
:040000 040000 5480780cb5e75c57122a621fc3bab0108c16be27 d97efd9cc0a465dff76bcd3a3c547f718f2a5345 M drivers
Reverting that from 3.5 makes the issue go away.
> Hi,
>
> when I use my usb ethernet adapter
>
> # > lsusb
> [...]
> Bus 002 Device 009: ID 9710:7830 MosChip Semiconductor MCS7830 10/100 Mbps Ethernet adapter
> [...]
>
> as port of an bridge
>
> > # brctl addbr br0
> > # brctl addif br0 eth0
> > # brctl addif br0 ue5
> > # ifconfig ue5 up
> > # ifconfig br0 up
>
> (Also does happen when eth0 is not part of the bridge, but the logs I
> had available were from that situation...)
>
> I constantly get messages showing the interface toggeling between
> disabled and forwarding state:
>
> Jul 23 07:40:50 elektra kernel: [ 1539.497337] br0: port 2(ue5) entered disabled state
> Jul 23 07:40:50 elektra kernel: [ 1539.554992] br0: port 2(ue5) entered forwarding state
> Jul 23 07:40:50 elektra kernel: [ 1539.555005] br0: port 2(ue5) entered forwarding state
> Jul 23 07:40:51 elektra kernel: [ 1540.496242] br0: port 2(ue5) entered disabled state
> Jul 23 07:40:51 elektra kernel: [ 1540.552534] br0: port 2(ue5) entered forwarding state
> Jul 23 07:40:51 elektra kernel: [ 1540.552548] br0: port 2(ue5) entered forwarding state
> Jul 23 07:40:52 elektra kernel: [ 1541.550413] br0: port 2(ue5) entered forwarding state
> Jul 23 07:40:53 elektra kernel: [ 1542.529672] br0: port 2(ue5) entered disabled state
> Jul 23 07:40:53 elektra kernel: [ 1542.587162] br0: port 2(ue5) entered forwarding state
> Jul 23 07:40:53 elektra kernel: [ 1542.587175] br0: port 2(ue5) entered forwarding state
> Jul 23 07:40:54 elektra kernel: [ 1543.585309] br0: port 2(ue5) entered forwarding state
> Jul 23 07:41:00 elektra kernel: [ 1549.360600] br0: port 2(ue5) entered disabled state
> Jul 23 07:41:00 elektra kernel: [ 1549.442998] br0: port 2(ue5) entered forwarding state
> Jul 23 07:41:00 elektra kernel: [ 1549.443011] br0: port 2(ue5) entered forwarding state
> Jul 23 07:41:01 elektra kernel: [ 1550.357686] br0: port 2(ue5) entered disabled state
> Jul 23 07:41:01 elektra kernel: [ 1550.408208] br0: port 2(ue5) entered forwarding state
> Jul 23 07:41:01 elektra kernel: [ 1550.408222] br0: port 2(ue5) entered forwarding state
> Jul 23 07:41:02 elektra kernel: [ 1551.407656] br0: port 2(ue5) entered forwarding state
> Jul 23 07:41:03 elektra kernel: [ 1552.401578] br0: port 2(ue5) entered disabled state
> Jul 23 07:41:03 elektra kernel: [ 1552.474773] br0: port 2(ue5) entered forwarding state
> Jul 23 07:41:03 elektra kernel: [ 1552.474786] br0: port 2(ue5) entered forwarding state
> Jul 23 07:41:04 elektra kernel: [ 1553.472487] br0: port 2(ue5) entered forwarding state
> Jul 23 07:41:05 elektra kernel: [ 1554.356138] br0: port 2(ue5) entered disabled state
> [...]
>
> This does (in the same situation, nothing else than the kernel changed)
> not happen with 3.4.5.
>
> Does anybody have an idea what the issue might be or do I need to bisect?
--
MfG,
Michael Leun
^ permalink raw reply
* Re: [GIT PULL nf-next] IPVS
From: David Miller @ 2012-07-23 23:35 UTC (permalink / raw)
To: horms; +Cc: pablo, lvs-devel, netdev, netfilter-devel, wensong, ja
In-Reply-To: <1343086141-9086-1-git-send-email-horms@verge.net.au>
From: Simon Horman <horms@verge.net.au>
Date: Tue, 24 Jul 2012 08:28:55 +0900
> please consider the following enhancements to IPVS for inclusion in 3.6.
The merge window has just openned, therefore any new work should have
been submitted and queued up already.
^ permalink raw reply
* [PATCH 5/5] ipvs: add pmtu_disc option to disable IP DF for TUN packets
From: Simon Horman @ 2012-07-23 23:29 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Horman
In-Reply-To: <1343086141-9086-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
Disabling PMTU discovery can increase the output packet
rate but some users have enough resources and prefer to fragment
than to drop traffic. By default, we copy the DF bit but if
pmtu_disc is disabled we do not send FRAG_NEEDED messages anymore.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/net/ip_vs.h | 11 +++++++++++
net/netfilter/ipvs/ip_vs_ctl.c | 8 ++++++++
net/netfilter/ipvs/ip_vs_xmit.c | 6 +++---
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 4b8f18f..ee75ccd 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -888,6 +888,7 @@ struct netns_ipvs {
unsigned int sysctl_sync_refresh_period;
int sysctl_sync_retries;
int sysctl_nat_icmp_send;
+ int sysctl_pmtu_disc;
/* ip_vs_lblc */
int sysctl_lblc_expiration;
@@ -974,6 +975,11 @@ static inline int sysctl_sync_sock_size(struct netns_ipvs *ipvs)
return ipvs->sysctl_sync_sock_size;
}
+static inline int sysctl_pmtu_disc(struct netns_ipvs *ipvs)
+{
+ return ipvs->sysctl_pmtu_disc;
+}
+
#else
static inline int sysctl_sync_threshold(struct netns_ipvs *ipvs)
@@ -1016,6 +1022,11 @@ static inline int sysctl_sync_sock_size(struct netns_ipvs *ipvs)
return 0;
}
+static inline int sysctl_pmtu_disc(struct netns_ipvs *ipvs)
+{
+ return 1;
+}
+
#endif
/*
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index d6d5cca..03d3fc6 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1801,6 +1801,12 @@ static struct ctl_table vs_vars[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "pmtu_disc",
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
#ifdef CONFIG_IP_VS_DEBUG
{
.procname = "debug_level",
@@ -3726,6 +3732,8 @@ static int __net_init ip_vs_control_net_init_sysctl(struct net *net)
ipvs->sysctl_sync_retries = clamp_t(int, DEFAULT_SYNC_RETRIES, 0, 3);
tbl[idx++].data = &ipvs->sysctl_sync_retries;
tbl[idx++].data = &ipvs->sysctl_nat_icmp_send;
+ ipvs->sysctl_pmtu_disc = 1;
+ tbl[idx++].data = &ipvs->sysctl_pmtu_disc;
ipvs->sysctl_hdr = register_net_sysctl(net, "net/ipv4/vs", tbl);
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index c2275ba..543a554 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -795,6 +795,7 @@ int
ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
struct ip_vs_protocol *pp)
{
+ struct netns_ipvs *ipvs = net_ipvs(skb_net(skb));
struct rtable *rt; /* Route to the other host */
__be32 saddr; /* Source for tunnel */
struct net_device *tdev; /* Device to other host */
@@ -830,10 +831,9 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
/* Copy DF, reset fragment offset and MF */
- df = old_iph->frag_off & htons(IP_DF);
+ df = sysctl_pmtu_disc(ipvs) ? old_iph->frag_off & htons(IP_DF) : 0;
- if ((old_iph->frag_off & htons(IP_DF) &&
- mtu < ntohs(old_iph->tot_len) && !skb_is_gso(skb))) {
+ if (df && mtu < ntohs(old_iph->tot_len) && !skb_is_gso(skb)) {
icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
IP_VS_DBG_RL("%s(): frag needed\n", __func__);
goto tx_error_put;
--
1.7.10.2.484.gcd07cc5
^ permalink raw reply related
* [PATCH 4/5] ipvs: implement passive PMTUD for IPIP packets
From: Simon Horman @ 2012-07-23 23:28 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Horman
In-Reply-To: <1343086141-9086-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
IPVS is missing the logic to update PMTU in routing
for its IPIP packets. We monitor the dst_mtu and can return
FRAG_NEEDED messages but if the tunneled packets get ICMP
error we can not rely on other traffic to save the lowest
MTU.
The following patch adds ICMP handling for IPIP
packets in incoming direction, from some remote host to
our local IP used as saddr in the outer header. By this
way we can forward any related ICMP traffic if it is for IPVS
TUN connection. For the special case of PMTUD we update the
routing and if client requested DF we can forward the
error.
To properly update the routing we have to bind
the cached route (dest->dst_cache) to the selected saddr
because ipv4_update_pmtu uses saddr for dst lookup.
Add IP_VS_RT_MODE_CONNECT flag to force such binding with
second route.
Update ip_vs_tunnel_xmit to provide IP_VS_RT_MODE_CONNECT
and change the code to copy DF. For now we prefer not to
force PMTU discovery (outer DF=1) because we don't have
configuration option to enable or disable PMTUD. As we
do not keep any packets to resend, we prefer not to
play games with packets without DF bit because the sender
is not informed when they are rejected.
Also, change ops->update_pmtu to be called only
for local clients because there is no point to update
MTU for input routes, in our case skb->dst->dev is lo.
It seems the code is copied from ipip.c where the skb
dst points to tunnel device.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_core.c | 76 +++++++++++++++++++++++++++++++++++++--
net/netfilter/ipvs/ip_vs_xmit.c | 79 ++++++++++++++++++++++++++++-------------
2 files changed, 128 insertions(+), 27 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index b54ecce..58918e2 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1303,7 +1303,8 @@ ip_vs_in_icmp(struct sk_buff *skb, int *related, unsigned int hooknum)
struct ip_vs_conn *cp;
struct ip_vs_protocol *pp;
struct ip_vs_proto_data *pd;
- unsigned int offset, ihl, verdict;
+ unsigned int offset, offset2, ihl, verdict;
+ bool ipip;
*related = 1;
@@ -1345,6 +1346,21 @@ ip_vs_in_icmp(struct sk_buff *skb, int *related, unsigned int hooknum)
net = skb_net(skb);
+ /* Special case for errors for IPIP packets */
+ ipip = false;
+ if (cih->protocol == IPPROTO_IPIP) {
+ if (unlikely(cih->frag_off & htons(IP_OFFSET)))
+ return NF_ACCEPT;
+ /* Error for our IPIP must arrive at LOCAL_IN */
+ if (!(skb_rtable(skb)->rt_flags & RTCF_LOCAL))
+ return NF_ACCEPT;
+ offset += cih->ihl * 4;
+ cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph);
+ if (cih == NULL)
+ return NF_ACCEPT; /* The packet looks wrong, ignore */
+ ipip = true;
+ }
+
pd = ip_vs_proto_data_get(net, cih->protocol);
if (!pd)
return NF_ACCEPT;
@@ -1358,11 +1374,14 @@ ip_vs_in_icmp(struct sk_buff *skb, int *related, unsigned int hooknum)
IP_VS_DBG_PKT(11, AF_INET, pp, skb, offset,
"Checking incoming ICMP for");
+ offset2 = offset;
offset += cih->ihl * 4;
ip_vs_fill_iphdr(AF_INET, cih, &ciph);
- /* The embedded headers contain source and dest in reverse order */
- cp = pp->conn_in_get(AF_INET, skb, &ciph, offset, 1);
+ /* The embedded headers contain source and dest in reverse order.
+ * For IPIP this is error for request, not for reply.
+ */
+ cp = pp->conn_in_get(AF_INET, skb, &ciph, offset, ipip ? 0 : 1);
if (!cp)
return NF_ACCEPT;
@@ -1376,6 +1395,57 @@ ip_vs_in_icmp(struct sk_buff *skb, int *related, unsigned int hooknum)
goto out;
}
+ if (ipip) {
+ __be32 info = ic->un.gateway;
+
+ /* Update the MTU */
+ if (ic->type == ICMP_DEST_UNREACH &&
+ ic->code == ICMP_FRAG_NEEDED) {
+ struct ip_vs_dest *dest = cp->dest;
+ u32 mtu = ntohs(ic->un.frag.mtu);
+
+ /* Strip outer IP and ICMP, go to IPIP header */
+ __skb_pull(skb, ihl + sizeof(_icmph));
+ offset2 -= ihl + sizeof(_icmph);
+ skb_reset_network_header(skb);
+ IP_VS_DBG(12, "ICMP for IPIP %pI4->%pI4: mtu=%u\n",
+ &ip_hdr(skb)->saddr, &ip_hdr(skb)->daddr, mtu);
+ rcu_read_lock();
+ ipv4_update_pmtu(skb, dev_net(skb->dev),
+ mtu, 0, 0, 0, 0);
+ rcu_read_unlock();
+ /* Client uses PMTUD? */
+ if (!(cih->frag_off & htons(IP_DF)))
+ goto ignore_ipip;
+ /* Prefer the resulting PMTU */
+ if (dest) {
+ spin_lock(&dest->dst_lock);
+ if (dest->dst_cache)
+ mtu = dst_mtu(dest->dst_cache);
+ spin_unlock(&dest->dst_lock);
+ }
+ if (mtu > 68 + sizeof(struct iphdr))
+ mtu -= sizeof(struct iphdr);
+ info = htonl(mtu);
+ }
+ /* Strip outer IP, ICMP and IPIP, go to IP header of
+ * original request.
+ */
+ __skb_pull(skb, offset2);
+ skb_reset_network_header(skb);
+ IP_VS_DBG(12, "Sending ICMP for %pI4->%pI4: t=%u, c=%u, i=%u\n",
+ &ip_hdr(skb)->saddr, &ip_hdr(skb)->daddr,
+ ic->type, ic->code, ntohl(info));
+ icmp_send(skb, ic->type, ic->code, info);
+ /* ICMP can be shorter but anyways, account it */
+ ip_vs_out_stats(cp, skb);
+
+ignore_ipip:
+ consume_skb(skb);
+ verdict = NF_STOLEN;
+ goto out;
+ }
+
/* do the statistics and put it back */
ip_vs_in_stats(cp, skb);
if (IPPROTO_TCP == cih->protocol || IPPROTO_UDP == cih->protocol)
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 65b616a..c2275ba 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -49,6 +49,7 @@ enum {
IP_VS_RT_MODE_RDR = 4, /* Allow redirect from remote daddr to
* local
*/
+ IP_VS_RT_MODE_CONNECT = 8, /* Always bind route to saddr */
};
/*
@@ -84,6 +85,42 @@ __ip_vs_dst_check(struct ip_vs_dest *dest, u32 rtos)
return dst;
}
+/* Get route to daddr, update *saddr, optionally bind route to saddr */
+static struct rtable *do_output_route4(struct net *net, __be32 daddr,
+ u32 rtos, int rt_mode, __be32 *saddr)
+{
+ struct flowi4 fl4;
+ struct rtable *rt;
+ int loop = 0;
+
+ memset(&fl4, 0, sizeof(fl4));
+ fl4.daddr = daddr;
+ fl4.saddr = (rt_mode & IP_VS_RT_MODE_CONNECT) ? *saddr : 0;
+ fl4.flowi4_tos = rtos;
+
+retry:
+ rt = ip_route_output_key(net, &fl4);
+ if (IS_ERR(rt)) {
+ /* Invalid saddr ? */
+ if (PTR_ERR(rt) == -EINVAL && *saddr &&
+ rt_mode & IP_VS_RT_MODE_CONNECT && !loop) {
+ *saddr = 0;
+ flowi4_update_output(&fl4, 0, rtos, daddr, 0);
+ goto retry;
+ }
+ IP_VS_DBG_RL("ip_route_output error, dest: %pI4\n", &daddr);
+ return NULL;
+ } else if (!*saddr && rt_mode & IP_VS_RT_MODE_CONNECT && fl4.saddr) {
+ ip_rt_put(rt);
+ *saddr = fl4.saddr;
+ flowi4_update_output(&fl4, 0, rtos, daddr, fl4.saddr);
+ loop++;
+ goto retry;
+ }
+ *saddr = fl4.saddr;
+ return rt;
+}
+
/* Get route to destination or remote server */
static struct rtable *
__ip_vs_get_out_rt(struct sk_buff *skb, struct ip_vs_dest *dest,
@@ -98,20 +135,13 @@ __ip_vs_get_out_rt(struct sk_buff *skb, struct ip_vs_dest *dest,
spin_lock(&dest->dst_lock);
if (!(rt = (struct rtable *)
__ip_vs_dst_check(dest, rtos))) {
- struct flowi4 fl4;
-
- memset(&fl4, 0, sizeof(fl4));
- fl4.daddr = dest->addr.ip;
- fl4.flowi4_tos = rtos;
- rt = ip_route_output_key(net, &fl4);
- if (IS_ERR(rt)) {
+ rt = do_output_route4(net, dest->addr.ip, rtos,
+ rt_mode, &dest->dst_saddr.ip);
+ if (!rt) {
spin_unlock(&dest->dst_lock);
- IP_VS_DBG_RL("ip_route_output error, dest: %pI4\n",
- &dest->addr.ip);
return NULL;
}
__ip_vs_dst_set(dest, rtos, dst_clone(&rt->dst), 0);
- dest->dst_saddr.ip = fl4.saddr;
IP_VS_DBG(10, "new dst %pI4, src %pI4, refcnt=%d, "
"rtos=%X\n",
&dest->addr.ip, &dest->dst_saddr.ip,
@@ -122,19 +152,17 @@ __ip_vs_get_out_rt(struct sk_buff *skb, struct ip_vs_dest *dest,
*ret_saddr = dest->dst_saddr.ip;
spin_unlock(&dest->dst_lock);
} else {
- struct flowi4 fl4;
+ __be32 saddr = htonl(INADDR_ANY);
- memset(&fl4, 0, sizeof(fl4));
- fl4.daddr = daddr;
- fl4.flowi4_tos = rtos;
- rt = ip_route_output_key(net, &fl4);
- if (IS_ERR(rt)) {
- IP_VS_DBG_RL("ip_route_output error, dest: %pI4\n",
- &daddr);
+ /* For such unconfigured boxes avoid many route lookups
+ * for performance reasons because we do not remember saddr
+ */
+ rt_mode &= ~IP_VS_RT_MODE_CONNECT;
+ rt = do_output_route4(net, daddr, rtos, rt_mode, &saddr);
+ if (!rt)
return NULL;
- }
if (ret_saddr)
- *ret_saddr = fl4.saddr;
+ *ret_saddr = saddr;
}
local = rt->rt_flags & RTCF_LOCAL;
@@ -331,6 +359,7 @@ ip_vs_dst_reset(struct ip_vs_dest *dest)
old_dst = dest->dst_cache;
dest->dst_cache = NULL;
dst_release(old_dst);
+ dest->dst_saddr.ip = 0;
}
#define IP_VS_XMIT_TUNNEL(skb, cp) \
@@ -771,7 +800,7 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
struct net_device *tdev; /* Device to other host */
struct iphdr *old_iph = ip_hdr(skb);
u8 tos = old_iph->tos;
- __be16 df = old_iph->frag_off;
+ __be16 df;
struct iphdr *iph; /* Our new IP header */
unsigned int max_headroom; /* The extra header space needed */
int mtu;
@@ -781,7 +810,8 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
if (!(rt = __ip_vs_get_out_rt(skb, cp->dest, cp->daddr.ip,
RT_TOS(tos), IP_VS_RT_MODE_LOCAL |
- IP_VS_RT_MODE_NON_LOCAL,
+ IP_VS_RT_MODE_NON_LOCAL |
+ IP_VS_RT_MODE_CONNECT,
&saddr)))
goto tx_error_icmp;
if (rt->rt_flags & RTCF_LOCAL) {
@@ -796,10 +826,11 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
IP_VS_DBG_RL("%s(): mtu less than 68\n", __func__);
goto tx_error_put;
}
- if (skb_dst(skb))
+ if (rt_is_output_route(skb_rtable(skb)))
skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
- df |= (old_iph->frag_off & htons(IP_DF));
+ /* Copy DF, reset fragment offset and MF */
+ df = old_iph->frag_off & htons(IP_DF);
if ((old_iph->frag_off & htons(IP_DF) &&
mtu < ntohs(old_iph->tot_len) && !skb_is_gso(skb))) {
--
1.7.10.2.484.gcd07cc5
^ permalink raw reply related
* [PATCH 3/5] ipvs: fixed sparse warning
From: Simon Horman @ 2012-07-23 23:28 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Claudiu Ghioc, Claudiu Ghioc, Simon Horman
In-Reply-To: <1343086141-9086-1-git-send-email-horms@verge.net.au>
From: Claudiu Ghioc <claudiughioc@gmail.com>
Removed the following sparse warnings, wether CONFIG_SYSCTL
is defined or not:
* warning: symbol 'ip_vs_control_net_init_sysctl' was not
declared. Should it be static?
* warning: symbol 'ip_vs_control_net_cleanup_sysctl' was
not declared. Should it be static?
Signed-off-by: Claudiu Ghioc <claudiu.ghioc@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_ctl.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 84444dd..d6d5cca 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3675,7 +3675,7 @@ static void ip_vs_genl_unregister(void)
* per netns intit/exit func.
*/
#ifdef CONFIG_SYSCTL
-int __net_init ip_vs_control_net_init_sysctl(struct net *net)
+static int __net_init ip_vs_control_net_init_sysctl(struct net *net)
{
int idx;
struct netns_ipvs *ipvs = net_ipvs(net);
@@ -3743,7 +3743,7 @@ int __net_init ip_vs_control_net_init_sysctl(struct net *net)
return 0;
}
-void __net_exit ip_vs_control_net_cleanup_sysctl(struct net *net)
+static void __net_exit ip_vs_control_net_cleanup_sysctl(struct net *net)
{
struct netns_ipvs *ipvs = net_ipvs(net);
@@ -3754,8 +3754,8 @@ void __net_exit ip_vs_control_net_cleanup_sysctl(struct net *net)
#else
-int __net_init ip_vs_control_net_init_sysctl(struct net *net) { return 0; }
-void __net_exit ip_vs_control_net_cleanup_sysctl(struct net *net) { }
+static int __net_init ip_vs_control_net_init_sysctl(struct net *net) { return 0; }
+static void __net_exit ip_vs_control_net_cleanup_sysctl(struct net *net) { }
#endif
--
1.7.10.2.484.gcd07cc5
^ permalink raw reply related
* [PATCH 2/5] ipvs: generalize app registration in netns
From: Simon Horman @ 2012-07-23 23:28 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Horman
In-Reply-To: <1343086141-9086-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
Get rid of the ftp_app pointer and allow applications
to be registered without adding fields in the netns_ipvs structure.
v2: fix coding style as suggested by Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/net/ip_vs.h | 5 ++--
net/netfilter/ipvs/ip_vs_app.c | 58 ++++++++++++++++++++++++++++++------------
net/netfilter/ipvs/ip_vs_ftp.c | 21 ++++-----------
3 files changed, 49 insertions(+), 35 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 95374d1..4b8f18f 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -808,8 +808,6 @@ struct netns_ipvs {
struct list_head rs_table[IP_VS_RTAB_SIZE];
/* ip_vs_app */
struct list_head app_list;
- /* ip_vs_ftp */
- struct ip_vs_app *ftp_app;
/* ip_vs_proto */
#define IP_VS_PROTO_TAB_SIZE 32 /* must be power of 2 */
struct ip_vs_proto_data *proto_data_table[IP_VS_PROTO_TAB_SIZE];
@@ -1179,7 +1177,8 @@ extern void ip_vs_service_net_cleanup(struct net *net);
* (from ip_vs_app.c)
*/
#define IP_VS_APP_MAX_PORTS 8
-extern int register_ip_vs_app(struct net *net, struct ip_vs_app *app);
+extern struct ip_vs_app *register_ip_vs_app(struct net *net,
+ struct ip_vs_app *app);
extern void unregister_ip_vs_app(struct net *net, struct ip_vs_app *app);
extern int ip_vs_bind_app(struct ip_vs_conn *cp, struct ip_vs_protocol *pp);
extern void ip_vs_unbind_app(struct ip_vs_conn *cp);
diff --git a/net/netfilter/ipvs/ip_vs_app.c b/net/netfilter/ipvs/ip_vs_app.c
index 64f9e8f..9713e6e 100644
--- a/net/netfilter/ipvs/ip_vs_app.c
+++ b/net/netfilter/ipvs/ip_vs_app.c
@@ -180,22 +180,38 @@ register_ip_vs_app_inc(struct net *net, struct ip_vs_app *app, __u16 proto,
}
-/*
- * ip_vs_app registration routine
- */
-int register_ip_vs_app(struct net *net, struct ip_vs_app *app)
+/* Register application for netns */
+struct ip_vs_app *register_ip_vs_app(struct net *net, struct ip_vs_app *app)
{
struct netns_ipvs *ipvs = net_ipvs(net);
- /* increase the module use count */
- ip_vs_use_count_inc();
+ struct ip_vs_app *a;
+ int err = 0;
+
+ if (!ipvs)
+ return ERR_PTR(-ENOENT);
mutex_lock(&__ip_vs_app_mutex);
- list_add(&app->a_list, &ipvs->app_list);
+ list_for_each_entry(a, &ipvs->app_list, a_list) {
+ if (!strcmp(app->name, a->name)) {
+ err = -EEXIST;
+ goto out_unlock;
+ }
+ }
+ a = kmemdup(app, sizeof(*app), GFP_KERNEL);
+ if (!a) {
+ err = -ENOMEM;
+ goto out_unlock;
+ }
+ INIT_LIST_HEAD(&a->incs_list);
+ list_add(&a->a_list, &ipvs->app_list);
+ /* increase the module use count */
+ ip_vs_use_count_inc();
+out_unlock:
mutex_unlock(&__ip_vs_app_mutex);
- return 0;
+ return err ? ERR_PTR(err) : a;
}
@@ -205,20 +221,29 @@ int register_ip_vs_app(struct net *net, struct ip_vs_app *app)
*/
void unregister_ip_vs_app(struct net *net, struct ip_vs_app *app)
{
- struct ip_vs_app *inc, *nxt;
+ struct netns_ipvs *ipvs = net_ipvs(net);
+ struct ip_vs_app *a, *anxt, *inc, *nxt;
+
+ if (!ipvs)
+ return;
mutex_lock(&__ip_vs_app_mutex);
- list_for_each_entry_safe(inc, nxt, &app->incs_list, a_list) {
- ip_vs_app_inc_release(net, inc);
- }
+ list_for_each_entry_safe(a, anxt, &ipvs->app_list, a_list) {
+ if (app && strcmp(app->name, a->name))
+ continue;
+ list_for_each_entry_safe(inc, nxt, &a->incs_list, a_list) {
+ ip_vs_app_inc_release(net, inc);
+ }
- list_del(&app->a_list);
+ list_del(&a->a_list);
+ kfree(a);
- mutex_unlock(&__ip_vs_app_mutex);
+ /* decrease the module use count */
+ ip_vs_use_count_dec();
+ }
- /* decrease the module use count */
- ip_vs_use_count_dec();
+ mutex_unlock(&__ip_vs_app_mutex);
}
@@ -586,5 +611,6 @@ int __net_init ip_vs_app_net_init(struct net *net)
void __net_exit ip_vs_app_net_cleanup(struct net *net)
{
+ unregister_ip_vs_app(net, NULL /* all */);
proc_net_remove(net, "ip_vs_app");
}
diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index b20b29c..ad70b7e 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -441,16 +441,10 @@ static int __net_init __ip_vs_ftp_init(struct net *net)
if (!ipvs)
return -ENOENT;
- app = kmemdup(&ip_vs_ftp, sizeof(struct ip_vs_app), GFP_KERNEL);
- if (!app)
- return -ENOMEM;
- INIT_LIST_HEAD(&app->a_list);
- INIT_LIST_HEAD(&app->incs_list);
- ipvs->ftp_app = app;
- ret = register_ip_vs_app(net, app);
- if (ret)
- goto err_exit;
+ app = register_ip_vs_app(net, &ip_vs_ftp);
+ if (IS_ERR(app))
+ return PTR_ERR(app);
for (i = 0; i < ports_count; i++) {
if (!ports[i])
@@ -464,9 +458,7 @@ static int __net_init __ip_vs_ftp_init(struct net *net)
return 0;
err_unreg:
- unregister_ip_vs_app(net, app);
-err_exit:
- kfree(ipvs->ftp_app);
+ unregister_ip_vs_app(net, &ip_vs_ftp);
return ret;
}
/*
@@ -474,10 +466,7 @@ err_exit:
*/
static void __ip_vs_ftp_exit(struct net *net)
{
- struct netns_ipvs *ipvs = net_ipvs(net);
-
- unregister_ip_vs_app(net, ipvs->ftp_app);
- kfree(ipvs->ftp_app);
+ unregister_ip_vs_app(net, &ip_vs_ftp);
}
static struct pernet_operations ip_vs_ftp_ops = {
--
1.7.10.2.484.gcd07cc5
^ permalink raw reply related
* [PATCH 1/5] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper
From: Simon Horman @ 2012-07-23 23:28 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov, Simon Horman
In-Reply-To: <1343086141-9086-1-git-send-email-horms@verge.net.au>
From: Julian Anastasov <ja@ssi.bg>
The FTP application indirectly depends on the
nf_conntrack_ftp helper for proper NAT support. If the
module is not loaded, IPVS can resize the packets for the
command connection, eg. PASV response but the SEQ adjustment
logic in ipv4_confirm is not called without helper.
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig
index f987138..8b2cffd 100644
--- a/net/netfilter/ipvs/Kconfig
+++ b/net/netfilter/ipvs/Kconfig
@@ -250,7 +250,8 @@ comment 'IPVS application helper'
config IP_VS_FTP
tristate "FTP protocol helper"
- depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT
+ depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT && \
+ NF_CONNTRACK_FTP
select IP_VS_NFCT
---help---
FTP is a protocol that transfers IP address and/or port number in
--
1.7.10.2.484.gcd07cc5
^ permalink raw reply related
* [GIT PULL nf-next] IPVS
From: Simon Horman @ 2012-07-23 23:28 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov
Hi Pablo,
please consider the following enhancements to IPVS for inclusion in 3.6.
----------------------------------------------------------------
The following changes since commit 9b70749e64132e17ab02239b82fcb4a2c55554d1:
niu: Change niu_rbr_fill() to use unlikely() to check niu_rbr_add_page() return value (2012-07-22 23:31:07 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git master
for you to fetch changes up to a805cfbcaaf819ab71a052d8a9d5d4c88cf2aba0:
ipvs: add pmtu_disc option to disable IP DF for TUN packets (2012-07-24 08:23:06 +0900)
----------------------------------------------------------------
Claudiu Ghioc (1):
ipvs: fixed sparse warning
Julian Anastasov (4):
ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper
ipvs: generalize app registration in netns
ipvs: implement passive PMTUD for IPIP packets
ipvs: add pmtu_disc option to disable IP DF for TUN packets
include/net/ip_vs.h | 16 ++++++--
net/netfilter/ipvs/Kconfig | 3 +-
net/netfilter/ipvs/ip_vs_app.c | 58 ++++++++++++++++++++--------
net/netfilter/ipvs/ip_vs_core.c | 76 +++++++++++++++++++++++++++++++++++--
net/netfilter/ipvs/ip_vs_ctl.c | 16 ++++++--
net/netfilter/ipvs/ip_vs_ftp.c | 21 +++--------
net/netfilter/ipvs/ip_vs_xmit.c | 83 ++++++++++++++++++++++++++++-------------
7 files changed, 204 insertions(+), 69 deletions(-)
^ permalink raw reply
* Re: [PATCH 2/2] ipv4: Change rt->rt_iif encoding.
From: David Miller @ 2012-07-23 23:14 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <20120723.160541.184307938805782289.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Mon, 23 Jul 2012 16:05:41 -0700 (PDT)
> From: Julian Anastasov <ja@ssi.bg>
> Date: Tue, 24 Jul 2012 02:11:38 +0300 (EEST)
>
>> skb_iif: ifindex of device we arrived on
>>
>> If it has hidden semantic may be we can make this
>> comment more specific, does it survive some loops?
>
> Strangely I hadn't noticed this, and after the email I just sent
> out I was going to look into adding such a value :-)
>
> Great!
>
> I'll respin my patches to use that thing, thanks Julian.
Hmmm, the problem is that when we decapsulate VLAN devices, we're left
with the parent device's index in skb->skb_iif.
That's what all of that "orig_dev" logic in __netif_receive_skb() is
all about. The skb->dev is rewritten by vlan_do_receive() for that
case.
I wonder if we should just get rid of all of that orig_dev logic and
simply update skb->skb_iif every time we hit the code starting at
label "another_round"
I'll keep looking into this.
^ permalink raw reply
* Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper
From: Simon Horman @ 2012-07-23 23:11 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Julian Anastasov, lvs-devel, netdev, netfilter-devel,
Wensong Zhang, Hans Schillstrom, Jesper Dangaard Brouer
In-Reply-To: <20120723173906.GA1430@1984>
On Mon, Jul 23, 2012 at 07:39:06PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Jul 23, 2012 at 03:48:18PM +0900, Simon Horman wrote:
> > On Thu, Jul 12, 2012 at 10:43:22PM +0300, Julian Anastasov wrote:
> > >
> > > Hello,
> > >
> > > On Thu, 12 Jul 2012, Pablo Neira Ayuso wrote:
> > >
> > > > On Wed, Jul 11, 2012 at 09:25:26AM +0900, Simon Horman wrote:
> > > > > From: Julian Anastasov <ja@ssi.bg>
> > > > >
> > > > > The FTP application indirectly depends on the
> > > > > nf_conntrack_ftp helper for proper NAT support. If the
> > > > > module is not loaded, IPVS can resize the packets for the
> > > > > command connection, eg. PASV response but the SEQ adjustment
> > > > > logic in ipv4_confirm is not called without helper.
> > > > >
> > > > > Signed-off-by: Julian Anastasov <ja@ssi.bg>
> > > > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > > > ---
> > > > > net/netfilter/ipvs/Kconfig | 3 ++-
> > > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig
> > > > > index f987138..8b2cffd 100644
> > > > > --- a/net/netfilter/ipvs/Kconfig
> > > > > +++ b/net/netfilter/ipvs/Kconfig
> > > > > @@ -250,7 +250,8 @@ comment 'IPVS application helper'
> > > > >
> > > > > config IP_VS_FTP
> > > > > tristate "FTP protocol helper"
> > > > > - depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT
> > > > > + depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT && \
> > > > > + NF_CONNTRACK_FTP
> > > >
> > > > If you require FTP NAT support, then this depends on NF_NAT_FTP
> > > > instead of NF_CONNTRACK_FTP.
> > >
> > > No, I just checked again, it works without nf_nat_ftp,
> > > only nf_nat, nf_conntrack_ftp and iptable_nat are needed.
> > > We use packet mangling part from nf_nat (nf_nat_mangle_tcp_packet).
> >
> > Is there a consensus on this?
>
> Fine with me, just wanted to make sure this what you wanted. Thanks
> Simon.
Thanks. I'll include this in a pull request after rebasing ipvs-next.
I plan to do that today.
^ permalink raw reply
* Re: [PATCH 2/2] ipv4: Change rt->rt_iif encoding.
From: David Miller @ 2012-07-23 23:05 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.1207240157120.2404@ja.ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Tue, 24 Jul 2012 02:11:38 +0300 (EEST)
> skb_iif: ifindex of device we arrived on
>
> If it has hidden semantic may be we can make this
> comment more specific, does it survive some loops?
Strangely I hadn't noticed this, and after the email I just sent
out I was going to look into adding such a value :-)
Great!
I'll respin my patches to use that thing, thanks Julian.
^ 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