* RE: [PATCH net-next 1/2] IP_GRE: Fix GRE_CSUM case.
From: Dmitry Kravkov @ 2013-02-25 11:40 UTC (permalink / raw)
To: Pravin B Shelar, davem@davemloft.net; +Cc: netdev@vger.kernel.org
In-Reply-To: <1361772305-1665-1-git-send-email-pshelar@nicira.com>
> -----Original Message-----
> From: Pravin B Shelar [mailto:pshelar@nicira.com]
> Sent: Monday, February 25, 2013 8:05 AM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; Dmitry Kravkov; Pravin B Shelar
> Subject: [PATCH net-next 1/2] IP_GRE: Fix GRE_CSUM case.
>
> commit "ip_gre: allow CSUM capable devices to handle packets"
> aa0e51cdda005cd37e2, broke GRE_CSUM case.
> GRE_CSUM needs checksum computed for inner packet. Therefore
> csum-calculation can not be offloaded if tunnel device requires
> GRE_CSUM. Following patch fixes it by computing inner packet checksum
> for GRE_CSUM type, for all other type of GRE devices csum is offloaded.
>
> CC: Dmitry Kravkov <dmitry@broadcom.com>
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
> ---
> net/ipv4/ip_gre.c | 12 ++++++++----
> 1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 5ef4da7..9b4996d 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -735,7 +735,7 @@ drop:
> return 0;
> }
>
> -static struct sk_buff *handle_offloads(struct sk_buff *skb)
> +static struct sk_buff *handle_offloads(struct ip_tunnel *tunnel, struct sk_buff *skb)
> {
> int err;
>
> @@ -745,8 +745,12 @@ static struct sk_buff *handle_offloads(struct sk_buff *skb)
> goto error;
> skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
> return skb;
> - }
> - if (skb->ip_summed != CHECKSUM_PARTIAL)
> + } else if (skb->ip_summed == CHECKSUM_PARTIAL &&
> + tunnel->parms.o_flags&GRE_CSUM) {
> + err = skb_checksum_help(skb);
> + if (unlikely(err))
> + goto error;
> + } else if (skb->ip_summed != CHECKSUM_PARTIAL)
> skb->ip_summed = CHECKSUM_NONE;
>
> return skb;
> @@ -776,7 +780,7 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
> int err;
> int pkt_len;
>
> - skb = handle_offloads(skb);
> + skb = handle_offloads(tunnel, skb);
> if (IS_ERR(skb)) {
> dev->stats.tx_dropped++;
> return NETDEV_TX_OK;
> --
> 1.7.1
Tested the series on two different devices
Acked-by: Dmitry Kravkov <dmitry@broadcom.com>
^ permalink raw reply
* RE: [PATCH net-next 2/2] Revert "ip_gre: propogate target device GSO capability to the tunnel device"
From: Dmitry Kravkov @ 2013-02-25 11:38 UTC (permalink / raw)
To: Pravin B Shelar, davem@davemloft.net; +Cc: netdev@vger.kernel.org
In-Reply-To: <1361772312-1698-1-git-send-email-pshelar@nicira.com>
> -----Original Message-----
> From: Pravin B Shelar [mailto:pshelar@nicira.com]
> Sent: Monday, February 25, 2013 8:05 AM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; Dmitry Kravkov; Pravin B Shelar
> Subject: [PATCH net-next 2/2] Revert "ip_gre: propogate target device GSO capability to the tunnel device"
>
> This reverts commit eb6b9a8cad65e820b145547844b108117cece3a0.
>
> Above commit limits GSO capability of gre device to just TSO, but
> software GRE-GSO is capable of handling all GSO capabilities.
>
> This patch also fixes following panic which reverted commit introduced:-
>
> BUG: unable to handle kernel NULL pointer dereference at 00000000000000a2
> IP: [<ffffffffa0680fd1>] ipgre_tunnel_bind_dev+0x161/0x1f0 [ip_gre]
> PGD 42bc19067 PUD 42bca9067 PMD 0
> Oops: 0000 [#1] SMP
> Pid: 2636, comm: ip Tainted: GF 3.8.0+ #83 Dell Inc. PowerEdge R620/0KCKR5
> RIP: 0010:[<ffffffffa0680fd1>] [<ffffffffa0680fd1>] ipgre_tunnel_bind_dev+0x161/0x1f0 [ip_gre]
> RSP: 0018:ffff88042bfcb708 EFLAGS: 00010246
> RAX: 00000000000005b6 RBX: ffff88042d2fa000 RCX: 0000000000000044
> RDX: 0000000000000018 RSI: 0000000000000078 RDI: 0000000000000060
> RBP: ffff88042bfcb748 R08: 0000000000000018 R09: 000000000000000c
> R10: 0000000000000020 R11: 000000000101010a R12: ffff88042d2fa800
> R13: 0000000000000000 R14: ffff88042d2fa800 R15: ffff88042cd7f650
> FS: 00007fa784f55700(0000) GS:ffff88043fd20000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00000000000000a2 CR3: 000000042d8b9000 CR4: 00000000000407e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process ip (pid: 2636, threadinfo ffff88042bfca000, task ffff88042d142a80)
> Stack:
> 0000000100000000 002f000000000000 0a01010100000000 000000000b010101
> ffff88042d2fa800 ffff88042d2fa000 ffff88042bfcb858 ffff88042f418c00
> ffff88042bfcb798 ffffffffa068199a ffff88042bfcb798 ffff88042d2fa830
> Call Trace:
> [<ffffffffa068199a>] ipgre_newlink+0xca/0x160 [ip_gre]
> [<ffffffff8143b692>] rtnl_newlink+0x532/0x5f0
> [<ffffffff8143b2fc>] ? rtnl_newlink+0x19c/0x5f0
> [<ffffffff81438978>] rtnetlink_rcv_msg+0x2c8/0x340
> [<ffffffff814386b0>] ? rtnetlink_rcv+0x40/0x40
> [<ffffffff814560f9>] netlink_rcv_skb+0xa9/0xd0
> [<ffffffff81438695>] rtnetlink_rcv+0x25/0x40
> [<ffffffff81455ddc>] netlink_unicast+0x1ac/0x230
> [<ffffffff81456a45>] netlink_sendmsg+0x265/0x380
> [<ffffffff814138c0>] sock_sendmsg+0xb0/0xe0
> [<ffffffff8141141e>] ? move_addr_to_kernel+0x4e/0x90
> [<ffffffff81420445>] ? verify_iovec+0x85/0xf0
> [<ffffffff81414ffd>] __sys_sendmsg+0x3fd/0x420
> [<ffffffff8114b701>] ? handle_mm_fault+0x251/0x3b0
> [<ffffffff8114f39f>] ? vma_link+0xcf/0xe0
> [<ffffffff81415239>] sys_sendmsg+0x49/0x90
> [<ffffffff814ffd19>] system_call_fastpath+0x16/0x1b
>
> CC: Dmitry Kravkov <dmitry@broadcom.com>
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
> ---
> net/ipv4/ip_gre.c | 10 ++--------
> 1 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 9b4996d..1517b6f 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1105,14 +1105,8 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
> tunnel->hlen = addend;
> /* TCP offload with GRE SEQ is not supported. */
> if (!(tunnel->parms.o_flags & GRE_SEQ)) {
> - /* device supports enc gso offload*/
> - if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
> - dev->features |= NETIF_F_TSO;
> - dev->hw_features |= NETIF_F_TSO;
> - } else {
> - dev->features |= NETIF_F_GSO_SOFTWARE;
> - dev->hw_features |= NETIF_F_GSO_SOFTWARE;
> - }
> + dev->features |= NETIF_F_GSO_SOFTWARE;
> + dev->hw_features |= NETIF_F_GSO_SOFTWARE;
> }
>
> return mtu;
Acked-by: Dmitry Kravkov <dmitry@broadcom.com>
^ permalink raw reply
* RE: [PATCH net-next 1/3] net: stmmac: add gmac autoneg set for SGMII, TBI, and RTBI
From: Byungho An @ 2013-02-25 10:28 UTC (permalink / raw)
To: 'Giuseppe CAVALLARO'
Cc: netdev, linux-kernel, davem, jeffrey.t.kirsher, kgene.kim, cpgs
In-Reply-To: <51277004.20702@st.com>
Hi Peppe,
I'll check patches and test on my side soon.
After that share with you.
Thank you.
> Hello Byungho
>
> sorry for my late reply. I'm attaching two patches built for net-next as
> we had clarified. I had written the first patch long time ago and on top
> of it I have added some part of your code below with some fixes (see also
> the comments inline below).
>
> This work is not yet completed but I do hope these two patches will help
> you to complete all. Unfortunately, I cannot do any tests because I have
> no HW that supports PCS. :-(
>
> In the second patch, take a look at the comment that reports the missing
> parts. For example, ethtool, SGMII etc.
>
> I wonder if you could review/test the two patches on your side.
> Also I hope you can improve all adding the missing features (that is what
> you were already doing).
>
> If you agree, you could also re-send *all* to the mailing list to be
> finally reviewed.
>
> On 1/25/2013 11:01 PM, Byungho An wrote:
> >
> > On 1/23/2013 1:43 PM, Giuseppe CAVALLARO write:
>
> [snip]
>
> >>
> > I modified this part like below
> >
> > @@ -1044,12 +1046,8 @@ static int stmmac_open(struct net_device *dev)
> > priv->hw->mac->core_init(priv->ioaddr);
> >
> > /* Enable auto-negotiation for SGMII, TBI or RTBI */
> > - if (interface == PHY_INTERFACE_MODE_SGMII ||
> > - interface == PHY_INTERFACE_MODE_TBI ||
> > - interface == PHY_INTERFACE_MODE_RTBI) {
> > - if (priv->phydev->autoneg)
> > - priv->hw->mac->set_autoneg(priv->ioaddr);
> > - }
> > + if (priv->dma_cap.pcs)
> > + priv->hw->mac->ctrl_ane(priv->ioaddr, 0);
> >
> > /* Request the IRQ lines */
> > ret = request_irq(dev->irq, stmmac_interrupt,
> >
> > As you may know, auto-negotiation is essential for SGMII, TBI, or RTBI
> > interface.
> >
>
> good, add it on top of the second patch.
>
> > And ctrl_ane funciont is like that
> >
> > @@ -311,6 +317,18 @@ static void dwmac1000_set_autoneg(void __iomem
> *ioaddr)
> > writel(value, ioaddr + GMAC_AN_CTRL);
> > }
> >
> > +static void dwmac1000_ctrl_ane(void __iomem *ioaddr, bool restart) {
> > + u32 value;
> > +
> > + value = readl(ioaddr + GMAC_AN_CTRL);
> > + /* auto negotiation enable and External Loopback enable */
> > + value = GMAC_AN_CTRL__ANE | GMAC_AN_CTRL__ELE;
> > +
> > + if (restart)
> > + value |= GMAC_AN_CTRL_RAN;
> > +
> > + writel(value, ioaddr + GMAC_AN_CTRL); }
> >
> > static const struct stmmac_ops dwmac1000_ops = {
> > .core_init = dwmac1000_core_init,
>
> well done and added in the second patch.
>
> [snip]
> > I've changed restart AN like below.
> >
> > @@ -610,6 +607,27 @@ static int stmmac_set_coalesce(struct net_device
> *dev,
> > return 0;
> > }
> >
> > +static int
> > +stmmac_nway_reset(struct net_device *netdev) {
> > + struct stmmac_priv *priv = netdev_priv(netdev);
> > + struct phy_device *phy = priv->phydev;
> > + int ret = 0;
> > +
> > + spin_lock(&priv->lock);
> > +
> > + if (netif_running(netdev)) {
> > + phy_stop(phy);
> > + phy_start(phy);
> > + ret = phy_start_aneg(phy);
> > + if (priv->dma_cap.pcs)
> > + priv->hw->mac->ctrl_ane(priv->ioaddr, true);
> > + }
> > +
> > + spin_unlock(&priv->lock);
> > + return ret;
> > +}
> > +
> > static const struct ethtool_ops stmmac_ethtool_ops = {
> > .begin = stmmac_check_if_running,
> > .get_drvinfo = stmmac_ethtool_getdrvinfo,
> >
> >
>
> we have to review this. I expect to have a new patch that includes the
> ethtool support but, at first glance, the stmmac_nway_reset should only
> call the ctrl_ane.
>
> pay attention that also some other ethtool functions have to be fixed for
> this support.
>
> [snip]
>
> > I think whenever link is changed, phy->state is changed and call
> > stmmac_adjust_link. It would update gmac's link.
> > I can get autonegotiation complete and link change irqs if we need
> > something we can add code in the handler but I'm not sure which one is
> need yet.
> > As of now I just added phy_state = PHY_CHANGELINK as a test code in
> > the link change irq handler.
> >
> > @@ -1624,8 +1629,43 @@ static irqreturn_t stmmac_interrupt(int irq,
> > void
> > *dev_id)
> >
priv->xstats.mmc_rx_csum_offload_irq_n++;
> > if (status & core_irq_receive_pmt_irq)
> > priv->xstats.irq_receive_pmt_irq_n++;
> > + if (status & core_irq_pcs_autoneg_complete)
> > + priv->core_pcs_an = true;
> > + if (status & core_irq_pcs_link_status_change) {
> > + priv->core_pcs_link_change = true;
> > + status = readl(priv->ioaddr +
> > GMAC_AN_STATUS);
> > + if (status & GMAC_AN_STATUS_LS)
> > + if ((priv->speed !=
> > + phy->speed) ||
> > (priv->oldduplex != phy->duplex))
> > + phy->state = PHY_CHANGELINK;
> > + /* for
> > test */
> > + }
> >
> > /* For LPI we need to save the tx status */
> > if (status & core_irq_tx_path_in_lpi_mode) {
> >
> > I have a question, how to hand over phy's irq number, as I analyzed
> > phydev->irq is irqlist and irqlist is like below but I can not find a
> > phydev->way to
> > set phy's irq number.
> > How to register or set priv->mii_irq or mdio_bus_data->irqs.
> >
> > if (mdio_bus_data->irqs)
> > irqlist = mdio_bus_data->irqs;
> > else
> > irqlist = priv->mii_irq;
>
> I had added something in my first patch that should be ok.
>
> > I added some defines for AN like below @@ -97,6 +97,20 @@ enum
> > power_event {
> > #define GMAC_TBI 0x000000d4 /* TBI extend status */
> > #define GMAC_GMII_STATUS 0x000000d8 /* S/R-GMII status */
> >
> > +/* AN Configuration defines */
> > +#define GMAC_AN_CTRL_RAN 0x00000200 /* Restart
Auto-Negotiation
> > */
> > +#define GMAC_AN_CTRL_ANE 0x00001000 /* Auto-Negotiation
Enable
> > */
> > +#define GMAC_AN_CTRL_ELE 0x00004000 /* External Loopback
Enable
> > */
> > +#define GMAC_AN_CTRL_ECD 0x00010000 /* Enable Comma Detect
*/
> > +#define GMAC_AN_CTRL_LR 0x00020000 /* Lock to Reference */
> > +#define GMAC_AN_CTRL_SGMRAL 0x00040000 /* SGMII RAL Control */
> > +
> > +/* AN Status defines */
> > +#define GMAC_AN_STATUS_LS 0x00000004 /* Link Status 0:down
1:up
> > */
> > +#define GMAC_AN_STATUS_ANA 0x00000008 /* Auto-Negotiation
> Ability
> > */
> > +#define GMAC_AN_STATUS_ANC 0x00000020 /* Auto-Negotiation
> Complete
> > */
> > +#define GMAC_AN_STATUS_ES 0x00000100 /* Extended Status */
> > +
> > /* GMAC Configuration defines */
> > #define GMAC_CONTROL_TC 0x01000000 /* Transmit Conf. in
> > RGMII/SGMII */
> > #define GMAC_CONTROL_WD 0x00800000 /* Disable Watchdog on
> > receive */
>
> ok, these are in the second patch
^ permalink raw reply
* Re: [PATCH] rndis_wlan: Remove redundant NULL check before kfree
From: Jussi Kivilinna @ 2013-02-25 10:02 UTC (permalink / raw)
To: Syam Sidhardhan; +Cc: linville, linux-wireless, netdev
In-Reply-To: <1361743543-27912-1-git-send-email-s.syam@samsung.com>
Acked-by: Jussi Kivilinna <jussi.kivilinna@mbnet.fi>
Quoting Syam Sidhardhan <syamsidhardh@gmail.com>:
> kfree on a NULL pointer is a no-op.
>
> Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
> ---
> drivers/net/wireless/rndis_wlan.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/rndis_wlan.c
> b/drivers/net/wireless/rndis_wlan.c
> index 525fd75..5f66b4e 100644
> --- a/drivers/net/wireless/rndis_wlan.c
> +++ b/drivers/net/wireless/rndis_wlan.c
> @@ -2839,8 +2839,7 @@ static void rndis_wlan_do_link_up_work(struct
> usbnet *usbdev)
> } else if (priv->infra_mode == NDIS_80211_INFRA_ADHOC)
> cfg80211_ibss_joined(usbdev->net, bssid, GFP_KERNEL);
>
> - if (info != NULL)
> - kfree(info);
> + kfree(info);
>
> priv->connected = true;
> memcpy(priv->bssid, bssid, ETH_ALEN);
> --
> 1.7.9.5
>
>
>
^ permalink raw reply
* Re: [PATCH] net: remove the unnecessary list_del
From: Gao feng @ 2013-02-25 9:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20130225.031256.202114640375754800.davem@davemloft.net>
On 2013/02/25 16:12, David Miller wrote:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Mon, 25 Feb 2013 16:01:24 +0800
>
>> I think it's better to make sure netdevice objects have valid pointers
>> by calling list_del in unregister_netdevice_many.
>
> Please, for the sake of understanding, read the commit below. And
> please make such necessary research in the future and reference any
> such commits in your proposed patch, as well as explaining why those
> commits in the past were wrong and why you're reverting of them is
> correct.
>
> Just saying "this isn't necessary" is a very disappointing explanation
> for something as very non-trivial as this is.
Get it, sorry for the noise :(
Will do more research before making patch.
Thanks, the commit below is very useful.
>
> Thank you.
>
> commit ceaaec98ad99859ac90ac6863ad0a6cd075d8e0e
> Author: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu Feb 17 22:59:19 2011 +0000
>
> net: deinit automatic LIST_HEAD
>
> commit 9b5e383c11b08784 (net: Introduce
> unregister_netdevice_many()) left an active LIST_HEAD() in
> rollback_registered(), with possible memory corruption.
>
> Even if device is freed without touching its unreg_list (and therefore
> touching the previous memory location holding LISTE_HEAD(single), better
> close the bug for good, since its really subtle.
>
> (Same fix for default_device_exit_batch() for completeness)
>
> Reported-by: Michal Hocko <mhocko@suse.cz>
> Tested-by: Michal Hocko <mhocko@suse.cz>
> Reported-by: Eric W. Biderman <ebiderman@xmission.com>
> Tested-by: Eric W. Biderman <ebiderman@xmission.com>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Ingo Molnar <mingo@elte.hu>
> CC: Octavian Purdila <opurdila@ixiacom.com>
> CC: stable <stable@kernel.org> [.33+]
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index a18c164..8ae6631 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5066,6 +5066,7 @@ static void rollback_registered(struct net_device *dev)
>
> list_add(&dev->unreg_list, &single);
> rollback_registered_many(&single);
> + list_del(&single);
> }
>
> unsigned long netdev_fix_features(unsigned long features, const char *name)
> @@ -6219,6 +6220,7 @@ static void __net_exit default_device_exit_batch(struct list_head *net_list)
> }
> }
> unregister_netdevice_many(&dev_kill_list);
> + list_del(&dev_kill_list);
> rtnl_unlock();
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH] net: remove the unnecessary list_del
From: David Miller @ 2013-02-25 8:12 UTC (permalink / raw)
To: gaofeng; +Cc: netdev
In-Reply-To: <512B1A54.1030004@cn.fujitsu.com>
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Mon, 25 Feb 2013 16:01:24 +0800
> I think it's better to make sure netdevice objects have valid pointers
> by calling list_del in unregister_netdevice_many.
Please, for the sake of understanding, read the commit below. And
please make such necessary research in the future and reference any
such commits in your proposed patch, as well as explaining why those
commits in the past were wrong and why you're reverting of them is
correct.
Just saying "this isn't necessary" is a very disappointing explanation
for something as very non-trivial as this is.
Thank you.
commit ceaaec98ad99859ac90ac6863ad0a6cd075d8e0e
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu Feb 17 22:59:19 2011 +0000
net: deinit automatic LIST_HEAD
commit 9b5e383c11b08784 (net: Introduce
unregister_netdevice_many()) left an active LIST_HEAD() in
rollback_registered(), with possible memory corruption.
Even if device is freed without touching its unreg_list (and therefore
touching the previous memory location holding LISTE_HEAD(single), better
close the bug for good, since its really subtle.
(Same fix for default_device_exit_batch() for completeness)
Reported-by: Michal Hocko <mhocko@suse.cz>
Tested-by: Michal Hocko <mhocko@suse.cz>
Reported-by: Eric W. Biderman <ebiderman@xmission.com>
Tested-by: Eric W. Biderman <ebiderman@xmission.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Ingo Molnar <mingo@elte.hu>
CC: Octavian Purdila <opurdila@ixiacom.com>
CC: stable <stable@kernel.org> [.33+]
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/net/core/dev.c b/net/core/dev.c
index a18c164..8ae6631 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5066,6 +5066,7 @@ static void rollback_registered(struct net_device *dev)
list_add(&dev->unreg_list, &single);
rollback_registered_many(&single);
+ list_del(&single);
}
unsigned long netdev_fix_features(unsigned long features, const char *name)
@@ -6219,6 +6220,7 @@ static void __net_exit default_device_exit_batch(struct list_head *net_list)
}
}
unregister_netdevice_many(&dev_kill_list);
+ list_del(&dev_kill_list);
rtnl_unlock();
}
^ permalink raw reply related
* Re: [PATCH] net: remove the unnecessary list_del
From: Gao feng @ 2013-02-25 8:01 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20130225.003931.1364384031279092217.davem@davemloft.net>
On 2013/02/25 13:39, David Miller wrote:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Mon, 25 Feb 2013 13:03:04 +0800
>
>> On 2013/02/25 12:54, David Miller wrote:
>>> From: Gao feng <gaofeng@cn.fujitsu.com>
>>> Date: Mon, 25 Feb 2013 12:41:56 +0800
>>>
>>>> These lists are used by unregister_netdevice_many,
>>>> they are local variables,will not be seen by others.
>>>> there is no need to delete them.
>>>>
>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>>>
>>> What about the devices on the list? The ones at the front and the
>>> rear of the list will have list pointers that point into no longer
>>> valid stack frame locations.
>>
>> These lists are only used by unregister_netdevice_many,
>> we don't access these lists again after unregister_netdevice_many,
>> so I think it doesn't have invalid stack frame locations problems.
>
> I do not see unregister_netdevice_many() list_del()'ing the devices
> on the list, therefore those netdevice objects have pointers into
> the stale stack frame.
>
Yes, I agree that this will make the netdevice objects have pointers
into the stale stack frame.
I check the codes,and found there are tons of codes that don't call
list_del after unregister_netdevice_many(). such as mroute_clean_tables,
ip6_tnl_destroy_tunnels...
I think it's better to make sure netdevice objects have valid pointers
by calling list_del in unregister_netdevice_many.
> You cannot make this change.
>
Will send another patch.
Thanks
^ permalink raw reply
* Good Day
From: Mr.Benjamin Tele @ 2013-02-25 7:44 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 43 bytes --]
PLEASE READ ATTACHED PROPOSAL FOR MORE INFO
[-- Attachment #2: Hello.pdf --]
[-- Type: application/pdf, Size: 264292 bytes --]
^ permalink raw reply
* Re: AF_VSOCK and the LSMs
From: Gerd Hoffmann @ 2013-02-25 7:29 UTC (permalink / raw)
To: Paul Moore; +Cc: Andy King, netdev, linux-security-module, selinux, Eric Paris
In-Reply-To: <2331260.82H25I6ITJ@sifl>
Hi,
>> I think perhaps this is the wrong layer at which to embed this. Think
>> of that structure as an ethernet header, with VMCI being ethernet; it's
>> what the device (and the hypervisor and peer) understand. So this
>> really cannot be changed.
>
> Hmmm, so can VMware/VMCI-enabled guests send vmci_datagram packets directly
> into the kernel? It isn't wrapped by things like AF_VSOCK? If that is the
> case, then yes, we'll probably need to add a thin wrapper struct to carry the
> security label; similar to the control packets but not quite, as we have data
> to deal with unlike the control packets. However, if vmci_datagram is an
> internal only structure, why not add the extra field?
vmci_datagram is part of the guest/host ABI I think.
Data flow looks like this:
(1) guest application opens a AF_VSOCK socket
(2) guest sends data as usual (say send syscall).
(3) vsock core hards over the packet to the transport layer
(only vmci atm, but we wanna add virtio here).
(4) transport layer passes data to the hypervisor (vmci uses a
virtual pci device for that, virtio will do the same).
(5) hypervisor forwards the data
(5a) cid=0 -- hypervisor handles the data itself.
(5b) cid=2 -- hypervisor hands over the data to a process on the host
(not sure how this works, Andy?)
(5c) cid=other -- hypervisor hands over data to the guest addressed
by the cid (==context id), where it travels back in a simliar way
(vmci virtual device -> vmci vsock transport ->
vsock core -> userspace app).
>> It's also not entirely clear to me how this will work in a heterogeneous
>> environments. What if there's a Linux guest running on a Windows host,
>> or vice-versa?
>
> I maybe missing something here, but VMCI never leaves the physical host system
> correct? It doesn't get tunneled over some external network does it?
Correct.
> Assuming it stays on the physical host system then we don't really care about
> a Windows host in this context do we? From a guests point of view it doesn't
> really matter, the kernel handles all of the labeling and access control; the
> guests create their AF_VSOCKS as they normally would.
Depends on where you wanna apply access control ...
If you want allow the *guest* kernel lsm allow to control which
applications are allowed to communicate using vsock and which are not,
then yes, it doesn't really matter which hypervisor is underneath as it
will not be involved in the first place. And I think the hooks should
be in the vsock core module not vmci, otherwise you'll have to exercise
the same again for each vsock transport layer we add. The transport
layer interface might need changes to support labeling & lsm hooks.
If you are looking at the host side (to restrict which guests are
allowed to talk to each other and which are not) lsm can't do much about
it as the data may not be passed to the host kernel in the first place ...
cheers,
Gerd
^ permalink raw reply
* Re: [PATCH v2 1/5] drivers: phy: add generic PHY framework
From: kishon @ 2013-02-25 6:41 UTC (permalink / raw)
To: Rob Landley
Cc: tony, linux, eballetbo, javier, balbi, gregkh, akpm, mchehab,
cesarb, davem, arnd, santosh.shilimkar, broonie, swarren,
linux-doc, linux-kernel, linux-arm-kernel, linux-omap, linux-usb,
netdev
In-Reply-To: <1361659480.11282.15@driftwood>
Hi,
On Sunday 24 February 2013 04:14 AM, Rob Landley wrote:
> On 02/18/2013 11:53:14 PM, Kishon Vijay Abraham I wrote:
>> The PHY framework provides a set of APIs for the PHY drivers to
>> create/destroy a PHY and APIs for the PHY users to obtain a reference
>> to the
>> PHY with or without using phandle. To obtain a reference to the PHY
>> without
>> using phandle, the platform specfic intialization code (say from board
>> file)
>> should have already called phy_bind with the binding information. The
>> binding
>> information consists of phy's device name, phy user device name and an
>> index.
>> The index is used when the same phy user binds to mulitple phys.
>
> Given that this has a separately selectable config option, I'm guessing
> that it's useful all by itself even in the absence of a driver using
Not really. It has to be thought of like a bridge that connects the
device (can be USB/SATA/..) and the PHY. The PHY driver will register
the PHY in this framework and the device controller driver can obtain a
reference to this PHY from this framework.
> this phy? (Or it gives user visibility to the phy buried in an E1000 or
> SATA drive or some such?)
The PHY and the device are generally independent entities and there has
to be a way to bind them in-order for the device controller to use the
PHY. For e.g., MUSB in OMAP4 uses a PHY which is different from PHY in
OMAP3 and it's going to be different from a PHY used in other SoCs. So
in-order for MUSB to use the correct PHY, there has to be a framework
that maintains the list of PHY and returns the correct PHY when a device
controller driver requests for it. So whenever the PHY driver gets
probed, it registers itself with this framework and then the MUSB can
get the reference to the PHY from this framework. The use of this
framework is more prevalent when there are multiple MUSB controllers
each using a different PHY in a single SoC.
This holds true for other device controllers as well.
>
>> +1. Introduction
>> +
>> +*PHY* is the abbreviation for physical layer. It is used to connect a
>> device
>> +to the physical medium e.g., the USB controller has a PHY to provide
>> functions
>> +such as serialization, de-serialization, encoding, decoding and is
>> responsible
>> +for obtaining the required data transmission rate. Note that some USB
>> +controller has PHY functionality embedded into it and others use an
>> external
>> +PHY. Other peripherals that uses a PHY include Wireless LAN, Ethernet,
>> +SATA etc.
>
> I've usually heard the word "transciever" used to describe these.
hmm.. IMO there is a thin line differentiating "transceiver" and "PHY"
and can be used interchangeably. Since it's been referred as "PHY" in
data sheets and TRMs, I preferred to call it PHY.
>
>> +The intention of creating this framework is to bring the phy drivers
>> spread
>> +all over the Linux kernel to drivers/phy to increase code re-use and to
>> +increase code maintainability.
>> +
>> +This framework will be of use only to devices that uses external PHY
>> (PHY
>> +functionality is not embedded within the controller).
>> +
>> +2. Creating the PHY
>> +
>> +The PHY driver should create the PHY in order for other peripheral
>> controllers
>> +to make use of it. The PHY framework provides 2 APIs to create the PHY.
>
> Given that a PHY is a chip (random example
> http://ark.intel.com/products/47620/Intel-82579LM-Gigabit-Ethernet-PHY),
> you seem to be saying that software should manifest a piece of hardware
> out of thin air through sheer willpower. I'm pretty sure I've
> misunderstood this phrasing.
>
>> +struct phy *phy_create(struct device *dev, struct phy_descriptor *desc);
>> +struct phy *devm_phy_create(struct device *dev, struct phy_descriptor
>> *desc);
>> +
>> +The PHY drivers can use one of the above 2 APIs to create the PHY by
>> passing
>
> Um, the driver should _bind_ to the phy, maybe? Allocate? Initialize?
There is actually difference between allocating/initializing and
binding. Binding is to bind the device controller with the PHY. My
previous explanation might help to clarify it.
>
>> +6. Destroying the PHY
>
> I've run drivers like that. I try not to, though.
>
>> +7. Current Status
>> +
>> +Currently only USB in OMAP is made to use this framework. However
>> using the
>> +USB PHY library cannot be completely removed because it is
>> intertwined with
>> +OTG. Once we move OTG out of PHY completely, using the old PHY
>> library can be
>> +completely removed. SATA in OMAP will also more likely use this new
>> framework
>> +and we should have a patch for it soon.
>
> Does this paragraph belong in the documentation? (Git commit, sure, but
> I've seen a lot of stale paragraphs like these hang around a
> surprisingly long time.)
hmm.. there are a few others who raised concern on having this
paragraph. I've planned to remove it in my next version.
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to handle packets
From: Pravin Shelar @ 2013-02-25 6:09 UTC (permalink / raw)
To: Dmitry Kravkov; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <1361725018.20403.6.camel@lb-tlvb-dmitry.il.broadcom.com>
On Sun, Feb 24, 2013 at 8:56 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> On Thu, 2013-02-21 at 23:19 -0800, pravin wrote:
>> On Thu, Feb 21, 2013 at 3:46 PM, pravin <pravin.shelar@gmail.com> wrote:
>> > On Thu, Feb 21, 2013 at 2:26 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> >> On Tue, 2013-02-19 at 15:03 -0800, pravin wrote:
>> >>> On Tue, Feb 19, 2013 at 11:20 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> >>> >
>> >>> >> -----Original Message-----
>> >>> >> From: pravin [mailto:pravin.shelar@gmail.com]
>> >>> >> Sent: Tuesday, February 19, 2013 8:28 PM
>> >>> >> To: Dmitry Kravkov
>> >>> >> Cc: davem@davemloft.net; netdev@vger.kernel.org
>> >>> >> Subject: Re: [PATCH net-next v2 1/2] ip_gre: allow CSUM capable devices to
>> >>> >> handle packets
>> >>> >>
>> >>> >> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com>
>> >>> >> wrote:
>> >>> >> > If device is not able to handle checksumming it will
>> >>> >> > be handled in dev_xmit
>> >>> >> >
>> >>> >> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>> >>> >> > ---
>> >>> >> > Changes from v1: fixed email address
>> >>> >> >
>> >>> >> > net/ipv4/ip_gre.c | 7 ++-----
>> >>> >> > 1 files changed, 2 insertions(+), 5 deletions(-)
>> >>> >> >
>> >>> >> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> >>> >> > index a56f118..cdc31ac 100644
>> >>> >> > --- a/net/ipv4/ip_gre.c
>> >>> >> > +++ b/net/ipv4/ip_gre.c
>> >>> >> > @@ -745,12 +745,9 @@ static struct sk_buff *handle_offloads(struct sk_buff
>> >>> >> *skb)
>> >>> >> > goto error;
>> >>> >> > skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
>> >>> >> > return skb;
>> >>> >> > - } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> >>> >> > - err = skb_checksum_help(skb);
>> >>> >> > - if (unlikely(err))
>> >>> >> > - goto error;
>> >>> >> > }
>> >>> >> > - skb->ip_summed = CHECKSUM_NONE;
>> >>> >> > + if (skb->ip_summed != CHECKSUM_PARTIAL)
>> >>> >> > + skb->ip_summed = CHECKSUM_NONE;
>> >>> >> >
>> >>> >> > return skb;
>> >>> >> >
>> >>> >> > --
>> >>> >> > 1.7.7.2
>> >>> >> >
>> >>> >> >
>> >>> >>
>> >>> >> This patch breaks GRE tunnel with GRE_CSUM. since GRE_CSUM need
>> >>> >> complete IP packet to checksum entire GRE payload.
>> >>> >
>> >>> > Testing for o_flags&GRE_CSUM does not look too hurt here, since it will be used in ipgre_tunnel_xmit() later on
>> >>> > This is the only problematic case, right?
>> >>> >
>> >>>
>> >>> It does not work for me. I have GRE device with csum on. Ping works
>> >>> fine but Netperf is not working.
>> >>> Looking at code, I am not sure how tcp will work if inner packet TCP
>> >>> checksum is calculated after GRE_CSUM calculation.
>> >>
>> >>
>> >> Fixes handling when CSUM or SEQ flags are set - via skb_checksum_help()
>> >> ---
>> >> Tested with each mode (separately)
>> >>
>> >> net/ipv4/ip_gre.c | 14 +++++++++++---
>> >> 1 files changed, 11 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> >> index 5ef4da7..0de54cd 100644
>> >> --- a/net/ipv4/ip_gre.c
>> >> +++ b/net/ipv4/ip_gre.c
>> >> @@ -735,7 +735,8 @@ drop:
>> >> return 0;
>> >> }
>> >>
>> >> -static struct sk_buff *handle_offloads(struct sk_buff *skb)
>> >> +static struct sk_buff *handle_offloads(struct ip_tunnel *tunnel,
>> >> + struct sk_buff *skb)
>> >> {
>> >> int err;
>> >>
>> >> @@ -745,8 +746,15 @@ static struct sk_buff *handle_offloads(struct
>> >> sk_buff *skb)
>> >> goto error;
>> >> skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
>> >> return skb;
>> >> + } else if (tunnel->parms.o_flags&(GRE_SEQ|GRE_CSUM) &&
>> >> + skb->ip_summed == CHECKSUM_PARTIAL) {
>> >> + err = skb_checksum_help(skb);
>> >> + if (unlikely(err))
>> >> + goto error;
>> >> }
>>
>> one more thing, there is no need to do csum for GRE_SEQ case.
> If csum is not performed TCP csum become incorrect for offload
> capable and incapable devices.
>
>
that csum computation is only required for GRE_CSUM case, not for GRE_SEQ.
I have send out patches to fix GRE issues, please have a look.
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next v2 2/2] ip_gre: propogate target device GSO capability to the tunnel device
From: Pravin Shelar @ 2013-02-25 6:07 UTC (permalink / raw)
To: Dmitry Kravkov; +Cc: pravin, davem, netdev
In-Reply-To: <1361724339.20403.2.camel@lb-tlvb-dmitry.il.broadcom.com>
On Sun, Feb 24, 2013 at 8:45 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
> On Thu, 2013-02-21 at 15:53 -0800, pravin wrote:
>> On Thu, Feb 21, 2013 at 2:30 PM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> > On Tue, 2013-02-19 at 10:39 -0800, pravin wrote:
>> >> On Mon, Feb 18, 2013 at 11:50 AM, Dmitry Kravkov <dmitry@broadcom.com> wrote:
>> >> >
>> >> > Signed-off-by: Dmitry Kravkov <dmitry@broadcom.com>
>> >> > ---
>> >> > Changes from v1: fixed email address
>> >> >
>> >> >
>> >> > net/ipv4/ip_gre.c | 10 ++++++++--
>> >> > 1 files changed, 8 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> >> > index cdc31ac..31bc941 100644
>> >> > --- a/net/ipv4/ip_gre.c
>> >> > +++ b/net/ipv4/ip_gre.c
>> >> > @@ -1103,8 +1103,14 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
>> >> > tunnel->hlen = addend;
>> >> > /* TCP offload with GRE SEQ is not supported. */
>> >> > if (!(tunnel->parms.o_flags & GRE_SEQ)) {
>> >> > - dev->features |= NETIF_F_GSO_SOFTWARE;
>> >> > - dev->hw_features |= NETIF_F_GSO_SOFTWARE;
>> >> > + /* device supports enc gso offload*/
>> >> > + if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
>> >> > + dev->features |= NETIF_F_TSO;
>> >> > + dev->hw_features |= NETIF_F_TSO;
>> >> > + } else {
>> >> > + dev->features |= NETIF_F_GSO_SOFTWARE;
>> >> > + dev->hw_features |= NETIF_F_GSO_SOFTWARE;
>> >> > + }
>> >> > }
>> >>
>> >> I am not sure about this change, Are you trying to limit GRE TSO to
>> >> just IPV4 in case of GRE offload in hardware?
>> >
>> >
>> > This mostly reverts previous patch, but also disables GSO when CSUM is
>> > set.
>> >
>> > ---
>> > net/ipv4/ip_gre.c | 14 ++++----------
>> > 1 files changed, 4 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
>> > index 0de54cd..aae2d4b 100644
>> > --- a/net/ipv4/ip_gre.c
>> > +++ b/net/ipv4/ip_gre.c
>> > @@ -1107,16 +1107,10 @@ static int ipgre_tunnel_bind_dev(struct
>> > net_device *dev)
>> > mtu = 68;
>> >
>> > tunnel->hlen = addend;
>> > - /* TCP offload with GRE SEQ is not supported. */
>> > - if (!(tunnel->parms.o_flags & GRE_SEQ)) {
>> > - /* device supports enc gso offload*/
>> > - if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
>> > - dev->features |= NETIF_F_TSO;
>> > - dev->hw_features |= NETIF_F_TSO;
>> > - } else {
>> > - dev->features |= NETIF_F_GSO_SOFTWARE;
>> > - dev->hw_features |= NETIF_F_GSO_SOFTWARE;
>> > - }
>> > + /* TCP offload with GRE SEQ or CSUM is not supported. */
>> > + if (!(tunnel->parms.o_flags & (GRE_SEQ|GRE_CSUM))) {
>> > + dev->features |= NETIF_F_GSO_SOFTWARE;
>> > + dev->hw_features |= NETIF_F_GSO_SOFTWARE;
>> > }
>> >
>> > return mtu;
>>
>> This narrows down definition of NETIF_F_GRE_GSO. Is it required for
>> hardware offload?
>
> This is required to prevent dev_hard_xmit() to perform soft GSO.
> W/o it device receives divided packets.
>
If NIC features are set correctly dev_hard_xmit() will not perform GSO.
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net-next 2/2] Revert "ip_gre: propogate target device GSO capability to the tunnel device"
From: Pravin B Shelar @ 2013-02-25 6:05 UTC (permalink / raw)
To: davem; +Cc: netdev, dmitry, Pravin B Shelar
This reverts commit eb6b9a8cad65e820b145547844b108117cece3a0.
Above commit limits GSO capability of gre device to just TSO, but
software GRE-GSO is capable of handling all GSO capabilities.
This patch also fixes following panic which reverted commit introduced:-
BUG: unable to handle kernel NULL pointer dereference at 00000000000000a2
IP: [<ffffffffa0680fd1>] ipgre_tunnel_bind_dev+0x161/0x1f0 [ip_gre]
PGD 42bc19067 PUD 42bca9067 PMD 0
Oops: 0000 [#1] SMP
Pid: 2636, comm: ip Tainted: GF 3.8.0+ #83 Dell Inc. PowerEdge R620/0KCKR5
RIP: 0010:[<ffffffffa0680fd1>] [<ffffffffa0680fd1>] ipgre_tunnel_bind_dev+0x161/0x1f0 [ip_gre]
RSP: 0018:ffff88042bfcb708 EFLAGS: 00010246
RAX: 00000000000005b6 RBX: ffff88042d2fa000 RCX: 0000000000000044
RDX: 0000000000000018 RSI: 0000000000000078 RDI: 0000000000000060
RBP: ffff88042bfcb748 R08: 0000000000000018 R09: 000000000000000c
R10: 0000000000000020 R11: 000000000101010a R12: ffff88042d2fa800
R13: 0000000000000000 R14: ffff88042d2fa800 R15: ffff88042cd7f650
FS: 00007fa784f55700(0000) GS:ffff88043fd20000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000000000a2 CR3: 000000042d8b9000 CR4: 00000000000407e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process ip (pid: 2636, threadinfo ffff88042bfca000, task ffff88042d142a80)
Stack:
0000000100000000 002f000000000000 0a01010100000000 000000000b010101
ffff88042d2fa800 ffff88042d2fa000 ffff88042bfcb858 ffff88042f418c00
ffff88042bfcb798 ffffffffa068199a ffff88042bfcb798 ffff88042d2fa830
Call Trace:
[<ffffffffa068199a>] ipgre_newlink+0xca/0x160 [ip_gre]
[<ffffffff8143b692>] rtnl_newlink+0x532/0x5f0
[<ffffffff8143b2fc>] ? rtnl_newlink+0x19c/0x5f0
[<ffffffff81438978>] rtnetlink_rcv_msg+0x2c8/0x340
[<ffffffff814386b0>] ? rtnetlink_rcv+0x40/0x40
[<ffffffff814560f9>] netlink_rcv_skb+0xa9/0xd0
[<ffffffff81438695>] rtnetlink_rcv+0x25/0x40
[<ffffffff81455ddc>] netlink_unicast+0x1ac/0x230
[<ffffffff81456a45>] netlink_sendmsg+0x265/0x380
[<ffffffff814138c0>] sock_sendmsg+0xb0/0xe0
[<ffffffff8141141e>] ? move_addr_to_kernel+0x4e/0x90
[<ffffffff81420445>] ? verify_iovec+0x85/0xf0
[<ffffffff81414ffd>] __sys_sendmsg+0x3fd/0x420
[<ffffffff8114b701>] ? handle_mm_fault+0x251/0x3b0
[<ffffffff8114f39f>] ? vma_link+0xcf/0xe0
[<ffffffff81415239>] sys_sendmsg+0x49/0x90
[<ffffffff814ffd19>] system_call_fastpath+0x16/0x1b
CC: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
net/ipv4/ip_gre.c | 10 ++--------
1 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 9b4996d..1517b6f 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1105,14 +1105,8 @@ static int ipgre_tunnel_bind_dev(struct net_device *dev)
tunnel->hlen = addend;
/* TCP offload with GRE SEQ is not supported. */
if (!(tunnel->parms.o_flags & GRE_SEQ)) {
- /* device supports enc gso offload*/
- if (tdev->hw_enc_features & NETIF_F_GRE_GSO) {
- dev->features |= NETIF_F_TSO;
- dev->hw_features |= NETIF_F_TSO;
- } else {
- dev->features |= NETIF_F_GSO_SOFTWARE;
- dev->hw_features |= NETIF_F_GSO_SOFTWARE;
- }
+ dev->features |= NETIF_F_GSO_SOFTWARE;
+ dev->hw_features |= NETIF_F_GSO_SOFTWARE;
}
return mtu;
--
1.7.1
^ permalink raw reply related
* [PATCH net-next 1/2] IP_GRE: Fix GRE_CSUM case.
From: Pravin B Shelar @ 2013-02-25 6:05 UTC (permalink / raw)
To: davem; +Cc: netdev, dmitry, Pravin B Shelar
commit "ip_gre: allow CSUM capable devices to handle packets"
aa0e51cdda005cd37e2, broke GRE_CSUM case.
GRE_CSUM needs checksum computed for inner packet. Therefore
csum-calculation can not be offloaded if tunnel device requires
GRE_CSUM. Following patch fixes it by computing inner packet checksum
for GRE_CSUM type, for all other type of GRE devices csum is offloaded.
CC: Dmitry Kravkov <dmitry@broadcom.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
net/ipv4/ip_gre.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 5ef4da7..9b4996d 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -735,7 +735,7 @@ drop:
return 0;
}
-static struct sk_buff *handle_offloads(struct sk_buff *skb)
+static struct sk_buff *handle_offloads(struct ip_tunnel *tunnel, struct sk_buff *skb)
{
int err;
@@ -745,8 +745,12 @@ static struct sk_buff *handle_offloads(struct sk_buff *skb)
goto error;
skb_shinfo(skb)->gso_type |= SKB_GSO_GRE;
return skb;
- }
- if (skb->ip_summed != CHECKSUM_PARTIAL)
+ } else if (skb->ip_summed == CHECKSUM_PARTIAL &&
+ tunnel->parms.o_flags&GRE_CSUM) {
+ err = skb_checksum_help(skb);
+ if (unlikely(err))
+ goto error;
+ } else if (skb->ip_summed != CHECKSUM_PARTIAL)
skb->ip_summed = CHECKSUM_NONE;
return skb;
@@ -776,7 +780,7 @@ static netdev_tx_t ipgre_tunnel_xmit(struct sk_buff *skb, struct net_device *dev
int err;
int pkt_len;
- skb = handle_offloads(skb);
+ skb = handle_offloads(tunnel, skb);
if (IS_ERR(skb)) {
dev->stats.tx_dropped++;
return NETDEV_TX_OK;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] net: remove the unnecessary list_del
From: David Miller @ 2013-02-25 5:39 UTC (permalink / raw)
To: gaofeng; +Cc: netdev
In-Reply-To: <512AF088.5040206@cn.fujitsu.com>
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Mon, 25 Feb 2013 13:03:04 +0800
> On 2013/02/25 12:54, David Miller wrote:
>> From: Gao feng <gaofeng@cn.fujitsu.com>
>> Date: Mon, 25 Feb 2013 12:41:56 +0800
>>
>>> These lists are used by unregister_netdevice_many,
>>> they are local variables,will not be seen by others.
>>> there is no need to delete them.
>>>
>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>>
>> What about the devices on the list? The ones at the front and the
>> rear of the list will have list pointers that point into no longer
>> valid stack frame locations.
>
> These lists are only used by unregister_netdevice_many,
> we don't access these lists again after unregister_netdevice_many,
> so I think it doesn't have invalid stack frame locations problems.
I do not see unregister_netdevice_many() list_del()'ing the devices
on the list, therefore those netdevice objects have pointers into
the stale stack frame.
You cannot make this change.
^ permalink raw reply
* Re: [PATCH] net: remove the unnecessary list_del
From: Gao feng @ 2013-02-25 5:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20130224.235411.1871885689914104471.davem@davemloft.net>
On 2013/02/25 12:54, David Miller wrote:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Mon, 25 Feb 2013 12:41:56 +0800
>
>> These lists are used by unregister_netdevice_many,
>> they are local variables,will not be seen by others.
>> there is no need to delete them.
>>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>
> What about the devices on the list? The ones at the front and the
> rear of the list will have list pointers that point into no longer
> valid stack frame locations.
These lists are only used by unregister_netdevice_many,
we don't access these lists again after unregister_netdevice_many,
so I think it doesn't have invalid stack frame locations problems.
^ permalink raw reply
* Re: [PATCH] net: remove the unnecessary list_del
From: David Miller @ 2013-02-25 4:54 UTC (permalink / raw)
To: gaofeng; +Cc: netdev
In-Reply-To: <1361767316-28583-1-git-send-email-gaofeng@cn.fujitsu.com>
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Mon, 25 Feb 2013 12:41:56 +0800
> These lists are used by unregister_netdevice_many,
> they are local variables,will not be seen by others.
> there is no need to delete them.
>
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
What about the devices on the list? The ones at the front and the
rear of the list will have list pointers that point into no longer
valid stack frame locations.
^ permalink raw reply
* [PATCH] net: remove the unnecessary list_del
From: Gao feng @ 2013-02-25 4:41 UTC (permalink / raw)
To: davem; +Cc: netdev, Gao feng
These lists are used by unregister_netdevice_many,
they are local variables,will not be seen by others.
there is no need to delete them.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
drivers/net/macvlan.c | 1 -
net/core/dev.c | 1 -
net/core/rtnetlink.c | 1 -
net/mac80211/iface.c | 1 -
4 files changed, 4 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index defcd8a..9b728f1 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -945,7 +945,6 @@ static int macvlan_device_event(struct notifier_block *unused,
list_for_each_entry_safe(vlan, next, &port->vlans, list)
vlan->dev->rtnl_link_ops->dellink(vlan->dev, &list_kill);
unregister_netdevice_many(&list_kill);
- list_del(&list_kill);
break;
case NETDEV_PRE_TYPE_CHANGE:
/* Forbid underlaying device to change its type. */
diff --git a/net/core/dev.c b/net/core/dev.c
index 18d8b5a..7c5e9d2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6172,7 +6172,6 @@ static void __net_exit default_device_exit_batch(struct list_head *net_list)
}
}
unregister_netdevice_many(&dev_kill_list);
- list_del(&dev_kill_list);
rtnl_unlock();
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d8aa20f..8e35210 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1613,7 +1613,6 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
ops->dellink(dev, &list_kill);
unregister_netdevice_many(&list_kill);
- list_del(&list_kill);
return 0;
}
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2c059e5..a799629 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1639,7 +1639,6 @@ void ieee80211_remove_interfaces(struct ieee80211_local *local)
}
mutex_unlock(&local->iflist_mtx);
unregister_netdevice_many(&unreg_list);
- list_del(&unreg_list);
list_for_each_entry_safe(sdata, tmp, &wdev_list, list) {
list_del(&sdata->list);
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH] net/pasemi: Fix missing coding style
From: David Miller @ 2013-02-25 2:22 UTC (permalink / raw)
To: joe; +Cc: syamsidhardh, netdev, olof
In-Reply-To: <1361758269.2281.14.camel@joe-AO722>
From: Joe Perches <joe@perches.com>
Date: Sun, 24 Feb 2013 18:11:09 -0800
> One additional line not two.
>
> You've mentioned it yourself in the past as something
> you seem to prefer.
>
> http://markmail.org/message/go7z57ztnl2nivpz#query:+page:1+mid:tgpy4tricd5yqktp+state:results
>
> and I don't much care except for consistency's sake.
Fair enough, I'll apply this patch, thanks Joe.
^ permalink raw reply
* Re: [PATCH] net/pasemi: Fix missing coding style
From: Joe Perches @ 2013-02-25 2:11 UTC (permalink / raw)
To: David Miller; +Cc: syamsidhardh, netdev, olof
In-Reply-To: <20130224.205216.341922963875740910.davem@davemloft.net>
On Sun, 2013-02-24 at 20:52 -0500, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Sun, 24 Feb 2013 17:23:29 -0800
>
> > On Sun, 2013-02-24 at 20:13 -0500, David Miller wrote:
> >> From: Syam Sidhardhan <syamsidhardh@gmail.com>
> >> Date: Mon, 25 Feb 2013 04:31:19 +0530
> >>
> >> > - } else
> >> > + } else {
> >> > freed = 2;
> >> > + }
> >>
> >> Single line statements do not need braces.
> >
> > I guess you didn't see the preceding block with braces.
> >
> > CodingStyle:
> >
> > This does not apply if only one branch of a conditional statement is a single
> > statement; in the latter case use braces in both branches:
>
> I did, and I don't think it's necessary in this case regardless
> of what CodingStyle says, do you really honestly think it increases
> readability enough to justify those two wasted vertical lines of
> screen space?
>
One additional line not two.
You've mentioned it yourself in the past as something
you seem to prefer.
http://markmail.org/message/go7z57ztnl2nivpz#query:+page:1+mid:tgpy4tricd5yqktp+state:results
and I don't much care except for consistency's sake.
^ permalink raw reply
* Re: [PATCH] net/pasemi: Fix missing coding style
From: David Miller @ 2013-02-25 1:52 UTC (permalink / raw)
To: joe; +Cc: syamsidhardh, netdev, olof
In-Reply-To: <1361755409.2281.9.camel@joe-AO722>
From: Joe Perches <joe@perches.com>
Date: Sun, 24 Feb 2013 17:23:29 -0800
> On Sun, 2013-02-24 at 20:13 -0500, David Miller wrote:
>> From: Syam Sidhardhan <syamsidhardh@gmail.com>
>> Date: Mon, 25 Feb 2013 04:31:19 +0530
>>
>> > - } else
>> > + } else {
>> > freed = 2;
>> > + }
>>
>> Single line statements do not need braces.
>
> I guess you didn't see the preceding block with braces.
>
> CodingStyle:
>
> This does not apply if only one branch of a conditional statement is a single
> statement; in the latter case use braces in both branches:
I did, and I don't think it's necessary in this case regardless
of what CodingStyle says, do you really honestly think it increases
readability enough to justify those two wasted vertical lines of
screen space?
^ permalink raw reply
* Re: [PATCH] net/pasemi: Fix missing coding style
From: Joe Perches @ 2013-02-25 1:23 UTC (permalink / raw)
To: David Miller; +Cc: syamsidhardh, netdev, olof
In-Reply-To: <20130224.201301.457851300548359313.davem@davemloft.net>
On Sun, 2013-02-24 at 20:13 -0500, David Miller wrote:
> From: Syam Sidhardhan <syamsidhardh@gmail.com>
> Date: Mon, 25 Feb 2013 04:31:19 +0530
>
> > - } else
> > + } else {
> > freed = 2;
> > + }
>
> Single line statements do not need braces.
I guess you didn't see the preceding block with braces.
CodingStyle:
This does not apply if only one branch of a conditional statement is a single
statement; in the latter case use braces in both branches:
^ permalink raw reply
* Re: [PATCH net-next] vmxnet3: fix ethtool ring buffer size setting
From: David Miller @ 2013-02-25 1:19 UTC (permalink / raw)
To: nhorman; +Cc: netdev, sbhatewara, pv-drivers
In-Reply-To: <1361565144-18301-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Fri, 22 Feb 2013 15:32:24 -0500
> Noticed that vmxnet3's get_ringparam function was returning the summation of all
> ring buffers on a NIC, rather than just the size of any one ring. This causes
> problems when a vmxnet3 instance has multiple queues, as ethtool, when setting
> ring parameters, first gets the current ring parameters to set the existing
> values in the set_ringparm commannd. The result is, that unless both rx and tx
> ring sizes are set in a single operation, which ever ring is not set will
> silently have its ring count multiplied by the number of queues on the NIC until
> it reaches a driver defined maxiumum value.
>
> Fix it by not multiplying the rx and tx ring sizes by the number of queues in
> the system, like every other driver. Tested by myself successfully.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] vmxnet3: make local function static
From: David Miller @ 2013-02-25 1:19 UTC (permalink / raw)
To: stephen; +Cc: netdev
In-Reply-To: <20130222102629.6690bdca@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 22 Feb 2013 10:26:29 -0800
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied.
^ permalink raw reply
* Re: [PATCH net] bnx2x: remove dead code and make local funcs static
From: David Miller @ 2013-02-25 1:16 UTC (permalink / raw)
To: eilong; +Cc: stephen, netdev, dmitry, yanivr
In-Reply-To: <1361737520.3557.3.camel@lb-tlvb-eilong.il.broadcom.com>
From: "Eilon Greenstein" <eilong@broadcom.com>
Date: Sun, 24 Feb 2013 22:25:20 +0200
> On Fri, 2013-02-22 at 10:01 -0800, Stephen Hemminger wrote:
>> Sparse warned about several functions that were unnecessarily global.
>> After making them static, discovered that several functions were actually never used.
>>
>> Compile tested only.
>>
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>
>
> Hi Stephen,
>
> Thanks for this patch. The "dead code" part is actually a missing call
> in case of PHY FW upgrade for the 84833 PHY - without it, a server
> reboot is required for the new PHY FW to take affect - but this reboot
> saving enhancement should wait until net-next is open, so I'm acking
> this patch and we will add the code back with the proper call once the
> net-next is open again.
>
> Acked-by: Eilon Greenstein <eilong@broadcomo.com>
Applied.
^ 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