Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] tcp: Restore RFC5961-compliant behavior for SYN packets
From: Neal Cardwell @ 2014-11-21 18:55 UTC (permalink / raw)
  To: Calvin Owens
  Cc: David S. Miller, Alexey Kuznetsov, James Morris, Eric Dumazet,
	kernel-team, Netdev, LKML
In-Reply-To: <1416524993-26228-1-git-send-email-calvinowens@fb.com>

On Thu, Nov 20, 2014 at 6:09 PM, Calvin Owens <calvinowens@fb.com> wrote:
> Commit c3ae62af8e755 ("tcp: should drop incoming frames without ACK
> flag set") was created to mitigate a security vulnerability in which a
> local attacker is able to inject data into locally-opened sockets by
> using TCP protocol statistics in procfs to quickly find the correct
> sequence number.
>
> This broke the RFC5961 requirement to send a challenge ACK in response
> to spurious RST packets, which was subsequently fixed by commit
> 7b514a886ba50 ("tcp: accept RST without ACK flag").
>
> Unfortunately, the RFC5961 requirement that spurious SYN packets be
> handled in a similar manner remains broken.
>
> RFC5961 section 4 states that:
>
>    ... the handling of the SYN in the synchronized state SHOULD be
>    performed as follows:
>
>    1) If the SYN bit is set, irrespective of the sequence number, TCP
>       MUST send an ACK (also referred to as challenge ACK) to the remote
>       peer:
>
>       <SEQ=SND.NXT><ACK=RCV.NXT><CTL=ACK>
>
>       After sending the acknowledgment, TCP MUST drop the unacceptable
>       segment and stop processing further.
>
>    By sending an ACK, the remote peer is challenged to confirm the loss
>    of the previous connection and the request to start a new connection.
>    A legitimate peer, after restart, would not have a TCB in the
>    synchronized state.  Thus, when the ACK arrives, the peer should send
>    a RST segment back with the sequence number derived from the ACK
>    field that caused the RST.
>
>    This RST will confirm that the remote peer has indeed closed the
>    previous connection.  Upon receipt of a valid RST, the local TCP
>    endpoint MUST terminate its connection.  The local TCP endpoint
>    should then rely on SYN retransmission from the remote end to
>    re-establish the connection.
>
> This patch lets SYN packets through the discard added in c3ae62af8e755,
> so that spurious SYN packets are properly dealt with as per the RFC.
>
> The challenge ACK is sent unconditionally and is rate-limited, so the
> original vulnerability is not reintroduced by this patch.
>
> Signed-off-by: Calvin Owens <calvinowens@fb.com>

Acked-by: Neal Cardwell <ncardwell@google.com>

neal

^ permalink raw reply

* Re: [PATCH 1/2] 8139too: Allow setting MTU larger than 1500
From: Alban @ 2014-11-21 18:57 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Aban Bedel, linux-kernel, netdev, Bjorn Helgaas, Benoit Taine,
	Eric W. Biederman, David S. Miller
In-Reply-To: <1416595911.7215.23.camel@decadent.org.uk>

On Fri, 21 Nov 2014 18:51:51 +0000
Ben Hutchings <ben@decadent.org.uk> wrote:

> On Fri, 2014-11-21 at 14:58 +0100, Alban wrote:
> > On Fri, 21 Nov 2014 00:34:34 +0000
> > Ben Hutchings <ben@decadent.org.uk> wrote:
> > 
> > > On Sat, 2014-11-08 at 12:48 +0100, Alban Bedel wrote:
> > > > Replace the default ndo_change_mtu callback with one that allow
> > > > setting MTU that the driver can handle.
> > > > 
> > > > Signed-off-by: Alban Bedel <albeu@free.fr>
> > > > ---
> > > >  drivers/net/ethernet/realtek/8139too.c | 13 ++++++++++++-
> > > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/net/ethernet/realtek/8139too.c
> > > > b/drivers/net/ethernet/realtek/8139too.c index 007b38c..8387de9
> > > > 100644 --- a/drivers/net/ethernet/realtek/8139too.c
> > > > +++ b/drivers/net/ethernet/realtek/8139too.c
> > > > @@ -185,6 +185,9 @@ static int debug = -1;
> > > >  /* max supported ethernet frame size -- must be at least
> > > > (dev->mtu+14+4).*/ #define MAX_ETH_FRAME_SIZE	1536
> > > >  
> > > > +/* max supported payload size */
> > > > +#define MAX_ETH_DATA_SIZE	(MAX_ETH_FRAME_SIZE -
> > > > ETH_HLEN - ETH_FCS_LEN)
> > > [...]
> > > 
> > > Does this maximum still allow for VLAN tags, or should it use
> > > VLAN_ETH_HLEN instead of ETH_HLEN?
> > 
> > That might well be as the VLAN code seems to assume that the
> > physical device can handle frames of MTU + VLAN_HLEN bytes. I can
> > fix it, but to me it seems like the VLAN code should be fixed to
> > respect the physical device MTU.
> 
> Drivers that support VLANs have to allow for at least one VLAN tag
> when validating the MTU.  This is obviously broken for multiple
> layers of VLAN tags, but those are the semantics we're stuck with.

Ok, I see. Should a I send a fix patch or redo a new version of this
patch?

Alban

^ permalink raw reply

* Re: pull request: wireless-next 2014-11-21
From: Larry Finger @ 2014-11-21 19:06 UTC (permalink / raw)
  To: John W. Linville, davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20141121183014.GA24984-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

On 11/21/2014 12:30 PM, John W. Linville wrote:
> Dave,
>
> Please pull this batch of updates intended for the 3.19 stream...
>

---snip---

> Larry Finger (3):
>        rtlwifi: Fix setting of tx descriptor for new trx flow
>        rtlwifi: Fix errors in descriptor manipulation
>        rtlwifi: rtl8192se: Fix connection problems

I probably do not understand the update process, but the above 3 patches need to 
be in kernel 3.18, not 3.19.

Thanks,

Larry

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 net-next v4 8/9] net: move vlan pop/push functions into common code
From: Jiri Pirko @ 2014-11-21 19:08 UTC (permalink / raw)
  To: Pravin Shelar
  Cc: Jiri Benc, netdev, David Miller, Jamal Hadi Salim, Tom Herbert,
	Eric Dumazet, Willem de Bruijn, Daniel Borkmann, mst, fw,
	Paul.Durrant, Thomas Graf, Cong Wang
In-Reply-To: <CALnjE+p3PtaOz7Ujp7KDK0-=giP-kzEDDDvw8=GCjJdcECYJEA@mail.gmail.com>

Fri, Nov 21, 2014 at 07:41:27PM CET, pshelar@nicira.com wrote:
>On Fri, Nov 21, 2014 at 10:05 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Fri, Nov 21, 2014 at 05:11:39PM CET, pshelar@nicira.com wrote:
>>>On Wed, Nov 19, 2014 at 5:05 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>> So it can be used from out of openvswitch code.
>>>> Did couple of cosmetic changes on the way, namely variable naming and
>>>> adding support for 8021AD proto.
>>>>
>>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>>> ---
>>>>
>>>> v3->v4:
>>>> - don't try to be smart and copy skb->mac_len manipulation as it is. It can be
>>>>   adjusted later on.
>>>
>>>I am fine with moving this code to generic library, as long as we can
>>>adjust the mac_len as offset for MPLS header, otherwise this breaks
>>>MPLS support.
>>
>> Pravin, I'm just moving the code without changing it in this aspect.
>> Maybe I'm missing something but can you tell me what you see wrong?
>> Thanks.
>>
>
>There is bug in the openvswitch code where vlan-pop does mac-length
>reset rather than subtracting VLAN_HLEN  from mac-len. MPLS code
>depends on this. Now this patch moves the bug to common code. I am
>fine with this patch and I can send fix to change code vlan-pop code
>to adjust mac_len later on.
>But I am worried about the comment made by Jiri Benc on earlier
>version of this patch where is mentioned that subtracting VLAN_HLEN
>can break common case for vlan-pop. In that case we can not change
>this common function. Jiri Benc, Can you point me to the code, so the
>we can work on solution to fix this issue.

Lets move to common code and fix this there with follow-up patches.
This common code should work for all users.

>
>>>
>>>> - fix error path in do_execute_actions as suggested by Pravin
>>>> v2->v3:
>>>> - used previously introduced __vlan_insert_tag helper
>>>> - used skb_push/pull to get skb->data into needed point
>>>> - fixed skb->mac_len computation in skb_vlan_push pointed out by Pravin
>>>> v1->v2:
>>>> - adjusted to fix recent ovs changes
>>>> - included change to use make_writable suggested by Eric
>>>>
>>>>  include/linux/skbuff.h    |  2 +
>>>>  net/core/skbuff.c         | 95 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>  net/openvswitch/actions.c | 86 +++++-------------------------------------
>>>>  3 files changed, 106 insertions(+), 77 deletions(-)
>>>>
>>>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>>>> index e045516..78c299f 100644
>>>> --- a/include/linux/skbuff.h
>>>> +++ b/include/linux/skbuff.h
>>>> @@ -2679,6 +2679,8 @@ unsigned int skb_gso_transport_seglen(const struct sk_buff *skb);
>>>>  struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
>>>>  struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
>>>>  int skb_ensure_writable(struct sk_buff *skb, int write_len);
>>>> +int skb_vlan_pop(struct sk_buff *skb);
>>>> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci);
>>>>
>>>>  struct skb_checksum_ops {
>>>>         __wsum (*update)(const void *mem, int len, __wsum wsum);
>>>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>>>> index d11bbe0..c906c5f 100644
>>>> --- a/net/core/skbuff.c
>>>> +++ b/net/core/skbuff.c
>>>> @@ -4163,6 +4163,101 @@ int skb_ensure_writable(struct sk_buff *skb, int write_len)
>>>>  }
>>>>  EXPORT_SYMBOL(skb_ensure_writable);
>>>>
>>>> +/* remove VLAN header from packet and update csum accordingly. */
>>>> +static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
>>>> +{
>>>> +       struct vlan_hdr *vhdr;
>>>> +       unsigned int offset = skb->data - skb_mac_header(skb);
>>>> +       int err;
>>>> +
>>>> +       __skb_push(skb, offset);
>>>> +       err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
>>>> +       if (unlikely(err))
>>>> +               goto pull;
>>>> +
>>>> +       skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
>>>> +
>>>> +       vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
>>>> +       *vlan_tci = ntohs(vhdr->h_vlan_TCI);
>>>> +
>>>> +       memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
>>>> +       __skb_pull(skb, VLAN_HLEN);
>>>> +
>>>> +       vlan_set_encap_proto(skb, vhdr);
>>>> +       skb->mac_header += VLAN_HLEN;
>>>> +
>>>> +       if (skb_network_offset(skb) < ETH_HLEN)
>>>> +               skb_set_network_header(skb, ETH_HLEN);
>>>> +
>>>> +       skb_reset_mac_len(skb);
>>>> +pull:
>>>> +       __skb_pull(skb, offset);
>>>> +
>>>> +       return err;
>>>> +}
>>>> +
>>>> +int skb_vlan_pop(struct sk_buff *skb)
>>>> +{
>>>> +       u16 vlan_tci;
>>>> +       __be16 vlan_proto;
>>>> +       int err;
>>>> +
>>>> +       if (likely(vlan_tx_tag_present(skb))) {
>>>> +               skb->vlan_tci = 0;
>>>> +       } else {
>>>> +               if (unlikely((skb->protocol != htons(ETH_P_8021Q) &&
>>>> +                             skb->protocol != htons(ETH_P_8021AD)) ||
>>>> +                            skb->len < VLAN_ETH_HLEN))
>>>> +                       return 0;
>>>> +
>>>> +               err = __skb_vlan_pop(skb, &vlan_tci);
>>>> +               if (err)
>>>> +                       return err;
>>>> +       }
>>>> +       /* move next vlan tag to hw accel tag */
>>>> +       if (likely((skb->protocol != htons(ETH_P_8021Q) &&
>>>> +                   skb->protocol != htons(ETH_P_8021AD)) ||
>>>> +                  skb->len < VLAN_ETH_HLEN))
>>>> +               return 0;
>>>> +
>>>> +       vlan_proto = skb->protocol;
>>>> +       err = __skb_vlan_pop(skb, &vlan_tci);
>>>> +       if (unlikely(err))
>>>> +               return err;
>>>> +
>>>> +       __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
>>>> +       return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(skb_vlan_pop);
>>>> +
>>>> +int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
>>>> +{
>>>> +       if (vlan_tx_tag_present(skb)) {
>>>> +               unsigned int offset = skb->data - skb_mac_header(skb);
>>>> +               int err;
>>>> +
>>>> +               /* __vlan_insert_tag expect skb->data pointing to mac header.
>>>> +                * So change skb->data before calling it and change back to
>>>> +                * original position later
>>>> +                */
>>>> +               __skb_push(skb, offset);
>>>> +               err = __vlan_insert_tag(skb, skb->vlan_proto,
>>>> +                                       vlan_tx_tag_get(skb));
>>>> +               if (err)
>>>> +                       return err;
>>>> +               skb->protocol = skb->vlan_proto;
>>>> +               skb->mac_len += VLAN_HLEN;
>>>> +               __skb_pull(skb, offset);
>>>> +
>>>> +               if (skb->ip_summed == CHECKSUM_COMPLETE)
>>>> +                       skb->csum = csum_add(skb->csum, csum_partial(skb->data
>>>> +                                       + (2 * ETH_ALEN), VLAN_HLEN, 0));
>>>> +       }
>>>> +       __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
>>>> +       return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(skb_vlan_push);
>>>> +
>>>>  /**
>>>>   * alloc_skb_with_frags - allocate skb with page frags
>>>>   *
>>>> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
>>>> index 7ffa377..4e05ea1 100644
>>>> --- a/net/openvswitch/actions.c
>>>> +++ b/net/openvswitch/actions.c
>>>> @@ -206,93 +206,27 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
>>>>         return 0;
>>>>  }
>>>>
>>>> -/* remove VLAN header from packet and update csum accordingly. */
>>>> -static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
>>>> -{
>>>> -       struct vlan_hdr *vhdr;
>>>> -       int err;
>>>> -
>>>> -       err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
>>>> -       if (unlikely(err))
>>>> -               return err;
>>>> -
>>>> -       skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
>>>> -
>>>> -       vhdr = (struct vlan_hdr *)(skb->data + ETH_HLEN);
>>>> -       *current_tci = vhdr->h_vlan_TCI;
>>>> -
>>>> -       memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
>>>> -       __skb_pull(skb, VLAN_HLEN);
>>>> -
>>>> -       vlan_set_encap_proto(skb, vhdr);
>>>> -       skb->mac_header += VLAN_HLEN;
>>>> -
>>>> -       if (skb_network_offset(skb) < ETH_HLEN)
>>>> -               skb_set_network_header(skb, ETH_HLEN);
>>>> -
>>>> -       /* Update mac_len for subsequent MPLS actions */
>>>> -       skb_reset_mac_len(skb);
>>>> -       return 0;
>>>> -}
>>>> -
>>>>  static int pop_vlan(struct sk_buff *skb, struct sw_flow_key *key)
>>>>  {
>>>> -       __be16 tci;
>>>>         int err;
>>>>
>>>> -       if (likely(vlan_tx_tag_present(skb))) {
>>>> -               skb->vlan_tci = 0;
>>>> -       } else {
>>>> -               if (unlikely(skb->protocol != htons(ETH_P_8021Q) ||
>>>> -                            skb->len < VLAN_ETH_HLEN))
>>>> -                       return 0;
>>>> -
>>>> -               err = __pop_vlan_tci(skb, &tci);
>>>> -               if (err)
>>>> -                       return err;
>>>> -       }
>>>> -       /* move next vlan tag to hw accel tag */
>>>> -       if (likely(skb->protocol != htons(ETH_P_8021Q) ||
>>>> -                  skb->len < VLAN_ETH_HLEN)) {
>>>> +       err = skb_vlan_pop(skb);
>>>> +       if (vlan_tx_tag_present(skb))
>>>> +               invalidate_flow_key(key);
>>>> +       else
>>>>                 key->eth.tci = 0;
>>>> -               return 0;
>>>> -       }
>>>> -
>>>> -       invalidate_flow_key(key);
>>>> -       err = __pop_vlan_tci(skb, &tci);
>>>> -       if (unlikely(err))
>>>> -               return err;
>>>> -
>>>> -       __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), ntohs(tci));
>>>> -       return 0;
>>>> +       return err;
>>>>  }
>>>>
>>>>  static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
>>>>                      const struct ovs_action_push_vlan *vlan)
>>>>  {
>>>> -       if (unlikely(vlan_tx_tag_present(skb))) {
>>>> -               u16 current_tag;
>>>> -
>>>> -               /* push down current VLAN tag */
>>>> -               current_tag = vlan_tx_tag_get(skb);
>>>> -
>>>> -               skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
>>>> -                                               current_tag);
>>>> -               if (!skb)
>>>> -                       return -ENOMEM;
>>>> -               /* Update mac_len for subsequent MPLS actions */
>>>> -               skb->mac_len += VLAN_HLEN;
>>>> -
>>>> -               if (skb->ip_summed == CHECKSUM_COMPLETE)
>>>> -                       skb->csum = csum_add(skb->csum, csum_partial(skb->data
>>>> -                                       + (2 * ETH_ALEN), VLAN_HLEN, 0));
>>>> -
>>>> +       if (vlan_tx_tag_present(skb))
>>>>                 invalidate_flow_key(key);
>>>> -       } else {
>>>> +       else
>>>>                 key->eth.tci = vlan->vlan_tci;
>>>> -       }
>>>> -       __vlan_hwaccel_put_tag(skb, vlan->vlan_tpid, ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
>>>> -       return 0;
>>>> +       return skb_vlan_push(skb, vlan->vlan_tpid,
>>>> +                            ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
>>>>  }
>>>>
>>>>  static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
>>>> @@ -858,8 +792,6 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
>>>>
>>>>                 case OVS_ACTION_ATTR_PUSH_VLAN:
>>>>                         err = push_vlan(skb, key, nla_data(a));
>>>> -                       if (unlikely(err)) /* skb already freed. */
>>>> -                               return err;
>>>>                         break;
>>>>
>>>>                 case OVS_ACTION_ATTR_POP_VLAN:
>>>> --
>>>> 1.9.3
>>>>

^ permalink raw reply

* Re: [PATCH 00/10] net: phy: add device-type abstraction
From: David Miller @ 2014-11-21 19:16 UTC (permalink / raw)
  To: johan; +Cc: f.fainelli, netdev, linux-kernel, bth, s.hauer, mark.rutland
In-Reply-To: <1416398363-32306-1-git-send-email-johan@kernel.org>

From: Johan Hovold <johan@kernel.org>
Date: Wed, 19 Nov 2014 12:59:13 +0100

> This series adds device and device-type abstractions to the micrel
> driver, and enables support for RMII-reference clock selection for
> KSZ8081 and KSZ8091 devices.

Series applied, thanks Johan.

^ permalink raw reply

* Re: [PATCH] l2tp_eth: allow to set a specific mac address
From: David Miller @ 2014-11-21 19:16 UTC (permalink / raw)
  To: lynxis; +Cc: netdev
In-Reply-To: <1416399879-3164-1-git-send-email-lynxis@fe80.eu>

From: Alexander Couzens <lynxis@fe80.eu>
Date: Wed, 19 Nov 2014 13:24:39 +0100

> Signed-off-by: Alexander Couzens <lynxis@fe80.eu>

Looks good, applied, thanks.

^ permalink raw reply

* Re: [patch net-next v4 0/9] sched: introduce vlan action
From: David Miller @ 2014-11-21 19:21 UTC (permalink / raw)
  To: jiri
  Cc: netdev, jhs, pshelar, therbert, edumazet, willemb, dborkman, mst,
	fw, Paul.Durrant, tgraf, cwang
In-Reply-To: <1416402303-25341-1-git-send-email-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 19 Nov 2014 14:04:54 +0100

> Please see the individual patches for info

Series applied, thanks Jiri.

Please work with others to resolve the MPLS header length subtraction
issue Pravin mentioned.

Thanks.

^ permalink raw reply

* Re: [PATCH net] net: Revert "net: avoid one atomic operation in skb_clone()"
From: Eric Dumazet @ 2014-11-21 19:29 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: Chris Mason, David Miller, netdev
In-Reply-To: <20141121180510.GA22112@kria>

On Fri, 2014-11-21 at 19:05 +0100, Sabrina Dubroca wrote:
> Hello Eric,
> 
> 2014-11-21, 09:04:11 -0800, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > Not sure what I was thinking, but doing anything after
> > releasing a refcount is suicidal or/and embarrassing.
> > 
> > By the time we set skb->fclone to SKB_FCLONE_FREE, another cpu
> > could have released last reference and freed whole skb.
> > 
> > We potentially corrupt memory or trap if CONFIG_DEBUG_PAGEALLOC is set.
> > 
> > Reported-by: Chris Mason <clm@fb.com>
> > Fixes: ce1a4ea3f1258 ("net: avoid one atomic operation in skb_clone()")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > ---
> >  net/core/skbuff.c |   23 ++++++-----------------
> >  1 file changed, 6 insertions(+), 17 deletions(-)
> > 
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index c16615bfb61e..be4c7deed971 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -552,20 +552,13 @@ static void kfree_skbmem(struct sk_buff *skb)
> >  	case SKB_FCLONE_CLONE:
> >  		fclones = container_of(skb, struct sk_buff_fclones, skb2);
> >  
> > -		/* Warning : We must perform the atomic_dec_and_test() before
> > -		 * setting skb->fclone back to SKB_FCLONE_FREE, otherwise
> > -		 * skb_clone() could set clone_ref to 2 before our decrement.
> > -		 * Anyway, if we are going to free the structure, no need to
> > -		 * rewrite skb->fclone.
> > +		/* The clone portion is available for
> > +		 * fast-cloning again.
> >  		 */
> > -		if (atomic_dec_and_test(&fclones->fclone_ref)) {
> > +		skb->fclone = SKB_FCLONE_UNAVAILABLE;
> 
> Shouldn't that be SKB_FCLONE_FREE ?

Absolutely :(

I'll send a v2, thanks !

^ permalink raw reply

* Re: [patch net-next v4 8/9] net: move vlan pop/push functions into common code
From: Jiri Benc @ 2014-11-21 19:29 UTC (permalink / raw)
  To: Pravin Shelar
  Cc: Jiri Pirko, netdev, David Miller, Jamal Hadi Salim, Tom Herbert,
	Eric Dumazet, Willem de Bruijn, Daniel Borkmann, mst, fw,
	Paul.Durrant, Thomas Graf, Cong Wang
In-Reply-To: <CALnjE+p3PtaOz7Ujp7KDK0-=giP-kzEDDDvw8=GCjJdcECYJEA@mail.gmail.com>

On Fri, 21 Nov 2014 10:41:27 -0800, Pravin Shelar wrote:
> There is bug in the openvswitch code where vlan-pop does mac-length
> reset rather than subtracting VLAN_HLEN  from mac-len. MPLS code
> depends on this. Now this patch moves the bug to common code. I am
> fine with this patch and I can send fix to change code vlan-pop code
> to adjust mac_len later on.
> But I am worried about the comment made by Jiri Benc on earlier
> version of this patch where is mentioned that subtracting VLAN_HLEN
> can break common case for vlan-pop. In that case we can not change
> this common function. Jiri Benc, Can you point me to the code, so the
> we can work on solution to fix this issue.

In such case, a skb that enters the __pop_vlan_tci function looks like
this:

+--------+--------+-------------+-----+---------+-------
| dstmac | srcmac | ETH_P_8021Q | tci | ETH_P_x | data
+--------+--------+-------------+-----+---------+-------
^                               ^
mac_header                      network_header

skb->protocol = ETH_P_8021Q
skb->mac_len = 14

After the function finishes, it's supposed to look like this:

                    +--------+--------+---------+-------
                    | dstmac | srcmac | ETH_P_x | data
                    +--------+--------+---------+-------
                    ^                           ^
                    mac_header                  network_header

skb->protocol = ETH_P_x
skb->mac_len = 14

Blindly decrementing mac_len wouldn't work here, you'd end up with
mac_len = 10. You'd either need to ensure this case doesn't happen (it
does with openvswitch currently) or detect this case.

 Jiri

-- 
Jiri Benc

^ permalink raw reply

* Re: [RFC] situation with csum_and_copy_... API
From: Al Viro @ 2014-11-21 19:39 UTC (permalink / raw)
  To: David Laight
  Cc: Eric Dumazet, David Miller, torvalds@linux-foundation.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	target-devel@vger.kernel.org, Nicholas A. Bellinger,
	Christoph Hellwig
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1C9F6930@AcuExch.aculab.com>

On Fri, Nov 21, 2014 at 05:42:55PM +0000, David Laight wrote:

> Callers of kernel_send/recvmsg() could easily be using a wrapper
> function that creates the 'msghdr'.
> When the want to send the remaining part of a buffer the old iterator
> will no longer be available - just the original iov and the required offset.

Er...  So why not copy a struct iov_iter to/from msg->msg_iter, then?
It's not as it had been particulary large - 5 words isn't much...

I'm not at all sure that _anything_ has valid reasons for draining iovecs.
Maintaining a struct iov_iter and modifying it is easy and actually faster...

Right now the main examples outside of net/* are due to unfortunate
limitations of ->sendmsg() - until now it had no way to be told that
desired data starts at offset.  With ->msg_iter it obviously becomes
possible...

^ permalink raw reply

* Re: [PATCH net] net: Revert "net: avoid one atomic operation in skb_clone()"
From: Chris Mason @ 2014-11-21 19:39 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Sabrina Dubroca, David Miller, netdev
In-Reply-To: <1416598161.8629.120.camel@edumazet-glaptop2.roam.corp.google.com>

On Fri, Nov 21, 2014 at 2:29 PM, Eric Dumazet <eric.dumazet@gmail.com> 
wrote:
> On Fri, 2014-11-21 at 19:05 +0100, Sabrina Dubroca wrote:
>>  Hello Eric,
>> 
>>  >
>>  > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>>  > index c16615bfb61e..be4c7deed971 100644
>>  > --- a/net/core/skbuff.c
>>  > +++ b/net/core/skbuff.c
>>  > @@ -552,20 +552,13 @@ static void kfree_skbmem(struct sk_buff 
>> *skb)
>>  >  	case SKB_FCLONE_CLONE:
>>  >  		fclones = container_of(skb, struct sk_buff_fclones, skb2);
>>  >
>>  > -		/* Warning : We must perform the atomic_dec_and_test() before
>>  > -		 * setting skb->fclone back to SKB_FCLONE_FREE, otherwise
>>  > -		 * skb_clone() could set clone_ref to 2 before our decrement.
>>  > -		 * Anyway, if we are going to free the structure, no need to
>>  > -		 * rewrite skb->fclone.
>>  > +		/* The clone portion is available for
>>  > +		 * fast-cloning again.
>>  >  		 */
>>  > -		if (atomic_dec_and_test(&fclones->fclone_ref)) {
>>  > +		skb->fclone = SKB_FCLONE_UNAVAILABLE;
>> 
>>  Shouldn't that be SKB_FCLONE_FREE ?
> 
> Absolutely :(
> 
> I'll send a v2, thanks !

v1 made it through the first pass here.  I'll switch to SKB_FCLONE_FREE 
and restart.

-chris

^ permalink raw reply

* Re: [RFC] situation with csum_and_copy_... API
From: Linus Torvalds @ 2014-11-21 19:40 UTC (permalink / raw)
  To: Al Viro
  Cc: David Laight, Eric Dumazet, David Miller, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, target-devel@vger.kernel.org,
	Nicholas A. Bellinger, Christoph Hellwig
In-Reply-To: <20141121193911.GU7996@ZenIV.linux.org.uk>

On Fri, Nov 21, 2014 at 11:39 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> I'm not at all sure that _anything_ has valid reasons for draining iovecs.
> Maintaining a struct iov_iter and modifying it is easy and actually faster...

For new code, I agree. But the whole "draining iovec's" is a fairly
common old model.

                  Linus

^ permalink raw reply

* Re: [PATCH net-next v2] packet: make packet_snd fail on len smaller than l2 header
From: David Miller @ 2014-11-21 19:43 UTC (permalink / raw)
  To: willemb; +Cc: netdev, eric.dumazet, dborkman
In-Reply-To: <1416420616-5029-1-git-send-email-willemb@google.com>

From: Willem de Bruijn <willemb@google.com>
Date: Wed, 19 Nov 2014 13:10:16 -0500

> From: Willem de Bruijn <willemb@google.com>
> 
> When sending packets out with PF_PACKET, SOCK_RAW, ensure that the
> packet is at least as long as the device's expected link layer header.
> This check already exists in tpacket_snd, but not in packet_snd.
> Also rate limit the warning in tpacket_snd.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: systemport: log RX buffer allocation and RX/TX DMA failures
From: David Miller @ 2014-11-21 19:46 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev
In-Reply-To: <1416421796-27657-2-git-send-email-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed, 19 Nov 2014 10:29:55 -0800

> To help troubleshoot heavy memory pressure conditions, add a bunch of
> statistics counter to log RX buffer allocation and RX/TX DMA mapping
> failures. These are reported like any other counters through the ethtool
> stats interface.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: bcmgenet: log RX buffer allocation and RX/TX dma failures
From: David Miller @ 2014-11-21 19:46 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev
In-Reply-To: <1416421796-27657-3-git-send-email-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed, 19 Nov 2014 10:29:56 -0800

> To help troubleshoot heavy memory pressure conditions, add a bunch of
> statistics counter to log RX buffer allocation and RX/TX DMA mapping
> failures. These are reported like any other counters through the ethtool
> stats interface.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH v4] ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg
From: David Miller @ 2014-11-21 19:46 UTC (permalink / raw)
  To: jbohac; +Cc: arnd, acme, netdev
In-Reply-To: <20141119220549.GA26133@midget.suse.cz>

From: Jiri Bohac <jbohac@suse.cz>
Date: Wed, 19 Nov 2014 23:05:49 +0100

> This fixes an old regression introduced by commit
> b0d0d915 (ipx: remove the BKL).
> 
> When a recvmsg syscall blocks waiting for new data, no data can be sent on the
> same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.
> 
> This breaks mars-nwe (NetWare emulator):
> - the ncpserv process reads the request using recvmsg
> - ncpserv forks and spawns nwconn
> - ncpserv calls a (blocking) recvmsg and waits for new requests
> - nwconn deadlocks in sendmsg on the same socket 
> 
> Commit b0d0d915 has simply replaced BKL locking with
> lock_sock/release_sock. Unlike now, BKL got unlocked while
> sleeping, so a blocking recvmsg did not block a concurrent
> sendmsg.
> 
> Only keep the socket locked while actually working with the socket data and
> release it prior to calling skb_recv_datagram(). 
> 
> 
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>

Applied.

^ permalink raw reply

* Re: [PATCHv3 net] openvswitch: Don't validate IPv6 label masks.
From: David Miller @ 2014-11-21 19:47 UTC (permalink / raw)
  To: joestringer; +Cc: netdev, linux-kernel, pshelar, dev
In-Reply-To: <1416434089-47062-1-git-send-email-joestringer@nicira.com>

From: Joe Stringer <joestringer@nicira.com>
Date: Wed, 19 Nov 2014 13:54:49 -0800

> When userspace doesn't provide a mask, OVS datapath generates a fully
> unwildcarded mask for the flow by copying the flow and setting all bits
> in all fields. For IPv6 label, this creates a mask that matches on the
> upper 12 bits, causing the following error:
> 
> openvswitch: netlink: Invalid IPv6 flow label value (value=ffffffff, max=fffff)
> 
> This patch ignores the label validation check for masks, avoiding this
> error.
> 
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> ---
> v3: Alternative approach.
>     Was "openvswitch: Fix mask generation for IPv6 labels."
> v2: OR lower 20 bits (upper 12 bits remain from earlier memdup)

Applied.

^ permalink raw reply

* [PATCH v2 net] net: Revert "net: avoid one atomic operation in skb_clone()"
From: Eric Dumazet @ 2014-11-21 19:47 UTC (permalink / raw)
  To: Sabrina Dubroca; +Cc: Chris Mason, David Miller, netdev
In-Reply-To: <1416598161.8629.120.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <edumazet@google.com>

Not sure what I was thinking, but doing anything after
releasing a refcount is suicidal or/and embarrassing.

By the time we set skb->fclone to SKB_FCLONE_FREE, another cpu
could have released last reference and freed whole skb.

We potentially corrupt memory or trap if CONFIG_DEBUG_PAGEALLOC is set.

Reported-by: Chris Mason <clm@fb.com>
Fixes: ce1a4ea3f1258 ("net: avoid one atomic operation in skb_clone()")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Sabrina Dubroca <sd@queasysnail.net>
---
v2: the revert went wrong, as SKB_FCLONE_UNAVAILABLE
    was later replaced by SKB_FCLONE_FREE, spotted by Sabrina Dubroca

 net/core/skbuff.c |   23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index c16615bfb61e..be4c7deed971 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -552,20 +552,13 @@ static void kfree_skbmem(struct sk_buff *skb)
 	case SKB_FCLONE_CLONE:
 		fclones = container_of(skb, struct sk_buff_fclones, skb2);
 
-		/* Warning : We must perform the atomic_dec_and_test() before
-		 * setting skb->fclone back to SKB_FCLONE_FREE, otherwise
-		 * skb_clone() could set clone_ref to 2 before our decrement.
-		 * Anyway, if we are going to free the structure, no need to
-		 * rewrite skb->fclone.
+		/* The clone portion is available for
+		 * fast-cloning again.
 		 */
-		if (atomic_dec_and_test(&fclones->fclone_ref)) {
+		skb->fclone = SKB_FCLONE_FREE;
+
+		if (atomic_dec_and_test(&fclones->fclone_ref))
 			kmem_cache_free(skbuff_fclone_cache, fclones);
-		} else {
-			/* The clone portion is available for
-			 * fast-cloning again.
-			 */
-			skb->fclone = SKB_FCLONE_FREE;
-		}
 		break;
 	}
 }
@@ -887,11 +880,7 @@ struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
 	if (skb->fclone == SKB_FCLONE_ORIG &&
 	    n->fclone == SKB_FCLONE_FREE) {
 		n->fclone = SKB_FCLONE_CLONE;
-		/* As our fastclone was free, clone_ref must be 1 at this point.
-		 * We could use atomic_inc() here, but it is faster
-		 * to set the final value.
-		 */
-		atomic_set(&fclones->fclone_ref, 2);
+		atomic_inc(&fclones->fclone_ref);
 	} else {
 		if (skb_pfmemalloc(skb))
 			gfp_mask |= __GFP_MEMALLOC;

^ permalink raw reply related

* Re: [PATCH net-next] bridge: make proxy arp configurable
From: David Miller @ 2014-11-21 19:49 UTC (permalink / raw)
  To: kyeyoonp; +Cc: shemming, netdev
In-Reply-To: <9dd8ca1857559a294f99c5849b51c8aa.squirrel@www.codeaurora.org>

From: kyeyoonp@codeaurora.org
Date: Wed, 19 Nov 2014 23:11:52 -0000

>> Please do not ever add empty lines at the end of files, GIT warns
>> about this when I try to apply your patch.
> 
> Hi Dave, I notice that this patch was never applied to "net-next".

I didn't apply it because it needs changes.

Please always look into patchwork to see what the state of a patch is,
if you had looked, you would have seen that I marked the patch as
"changed requested", which clearly states what the situation is.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next v2] net: sctp: keep owned chunk in destructor_arg instead of skb->cb
From: David Miller @ 2014-11-21 19:51 UTC (permalink / raw)
  To: dborkman; +Cc: linux-sctp, netdev
In-Reply-To: <1416444888-12778-1-git-send-email-dborkman@redhat.com>

From: Daniel Borkmann <dborkman@redhat.com>
Date: Thu, 20 Nov 2014 01:54:48 +0100

> It's just silly to hold the skb destructor argument around inside
> skb->cb[] as we currently do in SCTP.
> 
> Nowadays, we're sort of cheating on data accounting in the sense
> that due to commit 4c3a5bdae293 ("sctp: Don't charge for data in
> sndbuf again when transmitting packet"), we orphan the skb already
> in the SCTP output path, i.e. giving back charged data memory, and
> use a different destructor only to make sure the sk doesn't vanish
> on skb destruction time. Thus, cb[] is still valid here as we
> operate within the SCTP layer. (It's generally actually a big
> candidate for future rework, imho.)
> 
> However, storing the destructor in the cb[] can easily cause issues
> should an non sctp_packet_set_owner_w()'ed skb ever escape the SCTP
> layer, since cb[] may get overwritten by lower layers and thus can
> corrupt the chunk pointer. There are no such issues at present,
> but lets keep the chunk in destructor_arg, as this is the actual
> purpose for it.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
>  v1->v2:
>   - Only reworded commit message to make it more clear

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next v3 0/2] r8152: adjust rx functions
From: David Miller @ 2014-11-21 19:53 UTC (permalink / raw)
  To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-98-Taiwan-albertk@realtek.com>

From: Hayes Wang <hayeswang@realtek.com>
Date: Thu, 20 Nov 2014 10:29:04 +0800

> v3:
> For patch #1, remove unnecessary initialization for ret and
> unnecessary blank line in r8152_submit_rx().
> 
> v2:
> For patch #1, set actual_length to 0 before adding the rx to the
> list, when a error occurs.
> 
> For patch #2, change the flow. Stop submitting the rx if a error
> occurs, and add the remaining rx to the list for submitting later.
> 
> v1:
> Adjust some flows and codes which are relative to r8152_submit_rx()
> and rtl_start_rx().

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH net] net/mlx4_en: Add VXLAN ndo calls to the PF net device ops too
From: David Miller @ 2014-11-21 19:55 UTC (permalink / raw)
  To: ogerlitz; +Cc: netdev, fw, amirv, saeedm
In-Reply-To: <546D7913.7040508@mellanox.com>

From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Thu, 20 Nov 2014 07:16:03 +0200

> On 11/19/2014 10:11 PM, David Miller wrote:
>> Applied, -stable material? If so for what releases? In the future, if
>> you add an appropriated Fixes: tag I can figure most of this out
>> without asking you. Thanks.
> 
> 
> Yes, please! let the below commits two swim for a while in 3.18-rc and
> later push
> them to -stable >= 3.15, got it re the fixes: tag, thanks for the
> heads-up.
> 
> 9737c6a net/mlx4_en: Add VXLAN ndo calls to the PF net device ops too
> f4a1edd net/mlx4_en: Advertize encapsulation offloads features only
> when VXLAN tunnel is set

Ok, queued up for -stable.

^ permalink raw reply

* Re: mlx5: don't duplicate kvfree()
From: David Miller @ 2014-11-21 19:58 UTC (permalink / raw)
  To: viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn
  Cc: eli-VPRAkNaXOzVWk0Htik3J/w, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20141120081357.GI7996-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>

From: Al Viro <viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
Date: Thu, 20 Nov 2014 08:13:57 +0000

> Signed-off-by: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>

Applied.
--
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: mlx4: don't duplicate kvfree()
From: David Miller @ 2014-11-21 19:58 UTC (permalink / raw)
  To: viro; +Cc: amirv, linux-kernel, netdev
In-Reply-To: <20141120081538.GJ7996@ZenIV.linux.org.uk>

From: Al Viro <viro@ZenIV.linux.org.uk>
Date: Thu, 20 Nov 2014 08:15:38 +0000

> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] macvtap: advance iov iterator when needed in macvtap_put_user()
From: David Miller @ 2014-11-21 19:59 UTC (permalink / raw)
  To: jasowang; +Cc: netdev, linux-kernel, herbert
In-Reply-To: <1416472265-10151-1-git-send-email-jasowang@redhat.com>

From: Jason Wang <jasowang@redhat.com>
Date: Thu, 20 Nov 2014 16:31:05 +0800

> When mergeable buffer is used, vnet_hdr_sz is greater than sizeof struct
> virtio_net_hdr. So we need advance the iov iterators in this case.
> 
> Fixes 6c36d2e26cda1ad3e2c4b90dd843825fc62fe5b4 ("macvtap: Use iovec iterators")
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Applied, thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox