Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] net: bcmgenet: enable driver to work without a device tree
From: Petri Gynther @ 2014-12-01 21:13 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, David Miller
In-Reply-To: <CAGXr9JH2w1xoUwzzfdC4gi99iu3Eo4zkyWCsZi1b_x_gm+OKqg@mail.gmail.com>

Hi Florian,

Getting back to this (finally). I have made changes per your comments.
Sending a new patch shortly.

-- Petri

On Fri, Oct 10, 2014 at 12:46 PM, Petri Gynther <pgynther@google.com> wrote:
> Hi Florian,
>
> On Fri, Oct 10, 2014 at 11:59 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 10/10/2014 11:35 AM, Petri Gynther wrote:
>>> Broadcom 7xxx MIPS-based STB platforms do not use device trees.
>>> Modify bcmgenet driver so that it can be used on those platforms.
>>>
>>> Signed-off-by: Petri Gynther <pgynther@google.com>
>>> ---
>>
>> [snip]
>>
>>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
>>> index dbf524e..5191e3f 100644
>>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
>>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
>>> @@ -17,6 +17,17 @@
>>>  #include <linux/if_vlan.h>
>>>  #include <linux/phy.h>
>>>
>>> +struct bcmgenet_platform_data {
>>> +     void __iomem    *base_reg;
>>> +     int             irq0;
>>> +     int             irq1;
>>
>> Why would these members here? The platform device should provide those
>> as standard resources that the driver fetches using
>> platform_get_resource() and platform_get_irq().
>>
>
> I modeled this on struct bcmemac_platform_data that was used in the
> legacy BRCMSTB code.
> include/linux/brcmstb/brcmstb.h:
>
> struct bcmemac_platform_data {
>         /* used by the BSP code only */
>         uintptr_t               base_reg;
>         int                     irq0;
>         int                     irq1;
>
>         int                     phy_type;
>         int                     phy_id;
>         int                     phy_speed;
>         u8                      macaddr[ETH_ALEN];
> };
>
> The legacy BRCMSTB code stores all relevant GENET info in this struct
> and then creates the resources from that info:
>
> static void __init brcm_register_genet(int id, struct bcmemac_platform_data *pd)
> {
>         struct resource res[3];
>         struct platform_device *pdev;
>
>         memset(&res, 0, sizeof(res));
>         res[0].start = BPHYSADDR(pd->base_reg);
>         res[0].end = BPHYSADDR(pd->base_reg + 0x4fff);
>         res[0].flags = IORESOURCE_MEM;
>
>         res[1].start = res[1].end = pd->irq0;
>         res[1].flags = IORESOURCE_IRQ;
>
>         res[2].start = res[2].end = pd->irq1;
>         res[2].flags = IORESOURCE_IRQ;
>
>         brcm_alloc_macaddr(pd->macaddr);
>
>         pdev = platform_device_alloc("bcmgenet", id);
>         platform_device_add_resources(pdev, res, 3);
>         platform_device_add_data(pdev, pd, sizeof(*pd));
>         platform_device_add(pdev);
> }
>
>>> +     int             phy_type;
>>> +     int             phy_addr;
>>> +     int             phy_speed;
>>> +     u8              macaddr[ETH_ALEN];
>>> +     int             genet_version;
>>> +};
>>
>> I would rather we put this in include/linux/platform_data/bcmgenet.h
>> where it belongs.
>>
>
> I wasn't aware of the directory include/linux/platform_data/. Yes,
> that's where this belongs.
>
>>> +
>>>  /* total number of Buffer Descriptors, same for Rx/Tx */
>>>  #define TOTAL_DESC                           256
>>>
>>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
>>> index 9ff799a..e5ff913 100644
>>> --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
>>> +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
>>> @@ -157,6 +157,21 @@ static void bcmgenet_mii_setup(struct net_device *dev)
>>>       phy_print_status(phydev);
>>>  }
>>>
>>> +static int bcmgenet_moca_fphy_update(struct net_device *dev,
>>> +                                  struct fixed_phy_status *status)
>>> +{
>>> +     struct bcmgenet_priv *priv = netdev_priv(dev);
>>> +     struct phy_device *phydev = priv->phydev;
>>> +
>>> +     /*
>>> +      * MoCA daemon updates phydev->autoneg to reflect the link status.
>>> +      * Update MoCA fixed PHY status accordingly, so that the PHY state
>>> +      * machine becomes aware of the real link status.
>>> +      */
>>> +     status->link = phydev->autoneg;
>>> +     return 0;
>>> +}
>>
>> I don't want to see that in the upstream driver, please enable the link
>> interrupts like I suggested before and do not use the autoneg field at
>> all, which should require no MoCA daemon modifications.
>>
>
> I added debug printk's to bcmgenet_isr0 to check on UMAC_IRQ_LINK_UP
> and UMAC_IRQ_LINK_DOWN.
> I am not getting those interrupts on eth1 (MoCA) port when coax is
> removed/inserted.
> But, they do work on eth0.
>
> I'll modify init_umac() to enable those interrupts for MoCA port and retest.
>
>> [snip]
>>
>>>
>>>       priv->phydev = phydev;
>>> @@ -437,6 +464,104 @@ static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
>>>       return 0;
>>>  }
>>>
>>> +static int bcmgenet_mii_pd_init(struct bcmgenet_priv *priv)
>>> +{
>>> +     struct device *kdev = &priv->pdev->dev;
>>> +     struct bcmgenet_platform_data *pd = kdev->platform_data;
>>> +     struct mii_bus *mdio = priv->mii_bus;
>>> +     int phy_addr = pd->phy_addr;
>>> +     struct phy_device *phydev;
>>> +     int ret;
>>> +     int i;
>>> +
>>> +     /* Disable automatic MDIO bus scan */
>>> +     mdio->phy_mask = ~0;
>>> +
>>> +     /* Clear all the IRQ properties */
>>> +     if (mdio->irq)
>>> +             for (i = 0; i < PHY_MAX_ADDR; i++)
>>> +                     mdio->irq[i] = PHY_POLL;
>>> +
>>> +     /* Register the MDIO bus */
>>> +     ret = mdiobus_register(mdio);
>>> +     if (ret) {
>>> +             dev_err(kdev, "failed to register MDIO bus\n");
>>> +             return ret;
>>> +     }
>>> +
>>> +     /*
>>> +      * bcmgenet_platform_data needs to pass a valid PHY address for
>>> +      * internal/external PHY or -1 for MoCA PHY.
>>> +      */
>>> +     if (phy_addr >= 0 && phy_addr < PHY_MAX_ADDR) {
>>
>> We do too much low-level PHY device handling, and since you already have
>> the phy_type provided via platform_data, we can use that hint to do the
>> following:
>>
>> 1) an internal or external PHY with MDIO accesses should leave the bus
>> auto-probing on with the specified PHY address in the mdio bus phy_mask
>>
>> 2) a MoCA PHY or an external PHY with MDIO accesses disabled should use
>> the fixed-0 MII bus instead.
>>
>> This would look like this:
>>
>> if (pd->phy_type != PHY_INTERFACE_MODE_MOCA || pd->mdio_enabled)
>>         mdio->phy_mask = ~(1 << pd->phy_addr);
>>
>>         ...
>>         mdiobus_register()
>>
>>         priv->phydev = mdio->bus->phy_map[pd->phy_addr];
>>
>>         phydev->phy_flags |= mask;
>>
>> if (pd->phy_type == PHY_INTERFACE_MODE_MOCA || !pd->mdio_enabled)
>>         priv->phydev = fixed_phy_register(...);
>>
>> and in both cases, later on you do connect to the PHY device
>>
>> I can cook a patch to illustrate what I think this could look like since
>> I realize using pseudo-code to explain might not be the best thing.
>>
>
> I think I understand what you mean. I'll make a change.
>
>>> +             /*
>>> +              * 10/100/1000 Ethernet port with external or internal PHY.
>>> +              */
>>> +             phydev = get_phy_device(mdio, phy_addr, false);
>>> +             if (!phydev || IS_ERR(phydev)) {
>>> +                     dev_err(kdev, "failed to create PHY device\n");
>>> +                     mdiobus_unregister(mdio);
>>> +                     return 1;
>>> +             }
>>> +
>>> +             phydev->irq = PHY_POLL;
>>> +
>>> +             ret = phy_device_register(phydev);
>>> +             if (ret) {
>>> +                     dev_err(kdev, "failed to register PHY device\n");
>>> +                     phy_device_free(phydev);
>>> +                     mdiobus_unregister(mdio);
>>> +                     return 1;
>>> +             }
>>> +
>>> +             priv->phydev = phydev;
>>> +             priv->phy_interface = pd->phy_type;
>>> +     } else {
>>> +             /*
>>> +              * MoCA port with no MDIO-accessible PHY.
>>> +              * We need to use 1000/HD fixed PHY to represent the link layer.
>>> +              * MoCA daemon interacts with this PHY via ethtool.
>>> +              */
>>> +             struct fixed_phy_status moca_fphy_status = {
>>> +                     .link = 0,
>>> +                     .duplex = 0,
>>
>> This should be DUPLEX_FULL here, the link between GENET and the MoCA
>> Ethernet convergence layer is full-duplex by nature (despite we report
>> the PHY being half-duplex, which is a mistake in the downstream driver),
>> the MoCA medium on the coaxial cable is half-duplex though, but that is
>> handled by the MoCA HW.
>>
>> NB: I had issues in the past using a half-duplex link with the MoCA
>> ethernet convergence layer, causing various types of packet loss because
>> we use a simplified signaling internally in the hardware.
>>
>
> I picked this setting from 3.3 GENET driver. I'll test 1000/FULL on my
> platform to see if it works.
>
>>> +                     .speed = 1000,
>>> +                     .pause = 0,
>>> +                     .asym_pause = 0,
>>> +             };
>>> +
>>> +             phydev = fixed_phy_register(PHY_POLL, &moca_fphy_status, NULL);
>>> +             if (!phydev || IS_ERR(phydev)) {
>>> +                     dev_err(kdev, "failed to register fixed PHY device\n");
>>> +                     mdiobus_unregister(mdio);
>>> +                     return 1;
>>> +             }
>>> +
>>> +             phydev->autoneg = AUTONEG_DISABLE;
>>> +
>>> +             ret = fixed_phy_set_link_update(phydev,
>>> +                                             bcmgenet_moca_fphy_update);
>>> +             if (ret) {
>>> +                     dev_err(kdev, "failed to set fixed PHY link update\n");
>>> +             }
>>
>> Should not we propagate this error to the caller?
>
> Good catch. Yes.
>
>> --
>> Florian

^ permalink raw reply

* Re: [PATCH net-next] net: bcmgenet: enable driver to work without a device tree
From: Florian Fainelli @ 2014-12-01 21:20 UTC (permalink / raw)
  To: Petri Gynther; +Cc: netdev, David Miller, Kevin Cernekee
In-Reply-To: <CAGXr9JGgPD_LTCqbdNBgZi+pGboadRX0A3ZbdmAxp3rNBb12ww@mail.gmail.com>

On 01/12/14 13:13, Petri Gynther wrote:
> Hi Florian,
> 
> Getting back to this (finally). I have made changes per your comments.
> Sending a new patch shortly.

Ok, I am still a little bit hesitant in accepting these changes
considering that Kevin spent some time making a BMIPS multiplatform
kernel to work on 7425, 7435 [1]. Let's see how your changes look like,
and we can decide by then.

https://lwn.net/Articles/622947/

> 
> -- Petri
> 
> On Fri, Oct 10, 2014 at 12:46 PM, Petri Gynther <pgynther@google.com> wrote:
>> Hi Florian,
>>
>> On Fri, Oct 10, 2014 at 11:59 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>> On 10/10/2014 11:35 AM, Petri Gynther wrote:
>>>> Broadcom 7xxx MIPS-based STB platforms do not use device trees.
>>>> Modify bcmgenet driver so that it can be used on those platforms.
>>>>
>>>> Signed-off-by: Petri Gynther <pgynther@google.com>
>>>> ---
>>>
>>> [snip]
>>>
>>>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
>>>> index dbf524e..5191e3f 100644
>>>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
>>>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
>>>> @@ -17,6 +17,17 @@
>>>>  #include <linux/if_vlan.h>
>>>>  #include <linux/phy.h>
>>>>
>>>> +struct bcmgenet_platform_data {
>>>> +     void __iomem    *base_reg;
>>>> +     int             irq0;
>>>> +     int             irq1;
>>>
>>> Why would these members here? The platform device should provide those
>>> as standard resources that the driver fetches using
>>> platform_get_resource() and platform_get_irq().
>>>
>>
>> I modeled this on struct bcmemac_platform_data that was used in the
>> legacy BRCMSTB code.
>> include/linux/brcmstb/brcmstb.h:
>>
>> struct bcmemac_platform_data {
>>         /* used by the BSP code only */
>>         uintptr_t               base_reg;
>>         int                     irq0;
>>         int                     irq1;
>>
>>         int                     phy_type;
>>         int                     phy_id;
>>         int                     phy_speed;
>>         u8                      macaddr[ETH_ALEN];
>> };
>>
>> The legacy BRCMSTB code stores all relevant GENET info in this struct
>> and then creates the resources from that info:
>>
>> static void __init brcm_register_genet(int id, struct bcmemac_platform_data *pd)
>> {
>>         struct resource res[3];
>>         struct platform_device *pdev;
>>
>>         memset(&res, 0, sizeof(res));
>>         res[0].start = BPHYSADDR(pd->base_reg);
>>         res[0].end = BPHYSADDR(pd->base_reg + 0x4fff);
>>         res[0].flags = IORESOURCE_MEM;
>>
>>         res[1].start = res[1].end = pd->irq0;
>>         res[1].flags = IORESOURCE_IRQ;
>>
>>         res[2].start = res[2].end = pd->irq1;
>>         res[2].flags = IORESOURCE_IRQ;
>>
>>         brcm_alloc_macaddr(pd->macaddr);
>>
>>         pdev = platform_device_alloc("bcmgenet", id);
>>         platform_device_add_resources(pdev, res, 3);
>>         platform_device_add_data(pdev, pd, sizeof(*pd));
>>         platform_device_add(pdev);
>> }
>>
>>>> +     int             phy_type;
>>>> +     int             phy_addr;
>>>> +     int             phy_speed;
>>>> +     u8              macaddr[ETH_ALEN];
>>>> +     int             genet_version;
>>>> +};
>>>
>>> I would rather we put this in include/linux/platform_data/bcmgenet.h
>>> where it belongs.
>>>
>>
>> I wasn't aware of the directory include/linux/platform_data/. Yes,
>> that's where this belongs.
>>
>>>> +
>>>>  /* total number of Buffer Descriptors, same for Rx/Tx */
>>>>  #define TOTAL_DESC                           256
>>>>
>>>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
>>>> index 9ff799a..e5ff913 100644
>>>> --- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
>>>> +++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
>>>> @@ -157,6 +157,21 @@ static void bcmgenet_mii_setup(struct net_device *dev)
>>>>       phy_print_status(phydev);
>>>>  }
>>>>
>>>> +static int bcmgenet_moca_fphy_update(struct net_device *dev,
>>>> +                                  struct fixed_phy_status *status)
>>>> +{
>>>> +     struct bcmgenet_priv *priv = netdev_priv(dev);
>>>> +     struct phy_device *phydev = priv->phydev;
>>>> +
>>>> +     /*
>>>> +      * MoCA daemon updates phydev->autoneg to reflect the link status.
>>>> +      * Update MoCA fixed PHY status accordingly, so that the PHY state
>>>> +      * machine becomes aware of the real link status.
>>>> +      */
>>>> +     status->link = phydev->autoneg;
>>>> +     return 0;
>>>> +}
>>>
>>> I don't want to see that in the upstream driver, please enable the link
>>> interrupts like I suggested before and do not use the autoneg field at
>>> all, which should require no MoCA daemon modifications.
>>>
>>
>> I added debug printk's to bcmgenet_isr0 to check on UMAC_IRQ_LINK_UP
>> and UMAC_IRQ_LINK_DOWN.
>> I am not getting those interrupts on eth1 (MoCA) port when coax is
>> removed/inserted.
>> But, they do work on eth0.
>>
>> I'll modify init_umac() to enable those interrupts for MoCA port and retest.
>>
>>> [snip]
>>>
>>>>
>>>>       priv->phydev = phydev;
>>>> @@ -437,6 +464,104 @@ static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
>>>>       return 0;
>>>>  }
>>>>
>>>> +static int bcmgenet_mii_pd_init(struct bcmgenet_priv *priv)
>>>> +{
>>>> +     struct device *kdev = &priv->pdev->dev;
>>>> +     struct bcmgenet_platform_data *pd = kdev->platform_data;
>>>> +     struct mii_bus *mdio = priv->mii_bus;
>>>> +     int phy_addr = pd->phy_addr;
>>>> +     struct phy_device *phydev;
>>>> +     int ret;
>>>> +     int i;
>>>> +
>>>> +     /* Disable automatic MDIO bus scan */
>>>> +     mdio->phy_mask = ~0;
>>>> +
>>>> +     /* Clear all the IRQ properties */
>>>> +     if (mdio->irq)
>>>> +             for (i = 0; i < PHY_MAX_ADDR; i++)
>>>> +                     mdio->irq[i] = PHY_POLL;
>>>> +
>>>> +     /* Register the MDIO bus */
>>>> +     ret = mdiobus_register(mdio);
>>>> +     if (ret) {
>>>> +             dev_err(kdev, "failed to register MDIO bus\n");
>>>> +             return ret;
>>>> +     }
>>>> +
>>>> +     /*
>>>> +      * bcmgenet_platform_data needs to pass a valid PHY address for
>>>> +      * internal/external PHY or -1 for MoCA PHY.
>>>> +      */
>>>> +     if (phy_addr >= 0 && phy_addr < PHY_MAX_ADDR) {
>>>
>>> We do too much low-level PHY device handling, and since you already have
>>> the phy_type provided via platform_data, we can use that hint to do the
>>> following:
>>>
>>> 1) an internal or external PHY with MDIO accesses should leave the bus
>>> auto-probing on with the specified PHY address in the mdio bus phy_mask
>>>
>>> 2) a MoCA PHY or an external PHY with MDIO accesses disabled should use
>>> the fixed-0 MII bus instead.
>>>
>>> This would look like this:
>>>
>>> if (pd->phy_type != PHY_INTERFACE_MODE_MOCA || pd->mdio_enabled)
>>>         mdio->phy_mask = ~(1 << pd->phy_addr);
>>>
>>>         ...
>>>         mdiobus_register()
>>>
>>>         priv->phydev = mdio->bus->phy_map[pd->phy_addr];
>>>
>>>         phydev->phy_flags |= mask;
>>>
>>> if (pd->phy_type == PHY_INTERFACE_MODE_MOCA || !pd->mdio_enabled)
>>>         priv->phydev = fixed_phy_register(...);
>>>
>>> and in both cases, later on you do connect to the PHY device
>>>
>>> I can cook a patch to illustrate what I think this could look like since
>>> I realize using pseudo-code to explain might not be the best thing.
>>>
>>
>> I think I understand what you mean. I'll make a change.
>>
>>>> +             /*
>>>> +              * 10/100/1000 Ethernet port with external or internal PHY.
>>>> +              */
>>>> +             phydev = get_phy_device(mdio, phy_addr, false);
>>>> +             if (!phydev || IS_ERR(phydev)) {
>>>> +                     dev_err(kdev, "failed to create PHY device\n");
>>>> +                     mdiobus_unregister(mdio);
>>>> +                     return 1;
>>>> +             }
>>>> +
>>>> +             phydev->irq = PHY_POLL;
>>>> +
>>>> +             ret = phy_device_register(phydev);
>>>> +             if (ret) {
>>>> +                     dev_err(kdev, "failed to register PHY device\n");
>>>> +                     phy_device_free(phydev);
>>>> +                     mdiobus_unregister(mdio);
>>>> +                     return 1;
>>>> +             }
>>>> +
>>>> +             priv->phydev = phydev;
>>>> +             priv->phy_interface = pd->phy_type;
>>>> +     } else {
>>>> +             /*
>>>> +              * MoCA port with no MDIO-accessible PHY.
>>>> +              * We need to use 1000/HD fixed PHY to represent the link layer.
>>>> +              * MoCA daemon interacts with this PHY via ethtool.
>>>> +              */
>>>> +             struct fixed_phy_status moca_fphy_status = {
>>>> +                     .link = 0,
>>>> +                     .duplex = 0,
>>>
>>> This should be DUPLEX_FULL here, the link between GENET and the MoCA
>>> Ethernet convergence layer is full-duplex by nature (despite we report
>>> the PHY being half-duplex, which is a mistake in the downstream driver),
>>> the MoCA medium on the coaxial cable is half-duplex though, but that is
>>> handled by the MoCA HW.
>>>
>>> NB: I had issues in the past using a half-duplex link with the MoCA
>>> ethernet convergence layer, causing various types of packet loss because
>>> we use a simplified signaling internally in the hardware.
>>>
>>
>> I picked this setting from 3.3 GENET driver. I'll test 1000/FULL on my
>> platform to see if it works.
>>
>>>> +                     .speed = 1000,
>>>> +                     .pause = 0,
>>>> +                     .asym_pause = 0,
>>>> +             };
>>>> +
>>>> +             phydev = fixed_phy_register(PHY_POLL, &moca_fphy_status, NULL);
>>>> +             if (!phydev || IS_ERR(phydev)) {
>>>> +                     dev_err(kdev, "failed to register fixed PHY device\n");
>>>> +                     mdiobus_unregister(mdio);
>>>> +                     return 1;
>>>> +             }
>>>> +
>>>> +             phydev->autoneg = AUTONEG_DISABLE;
>>>> +
>>>> +             ret = fixed_phy_set_link_update(phydev,
>>>> +                                             bcmgenet_moca_fphy_update);
>>>> +             if (ret) {
>>>> +                     dev_err(kdev, "failed to set fixed PHY link update\n");
>>>> +             }
>>>
>>> Should not we propagate this error to the caller?
>>
>> Good catch. Yes.
>>
>>> --
>>> Florian

^ permalink raw reply

* [PATCH net-next] hyperv: Add support for vNIC hot removal
From: Haiyang Zhang @ 2014-12-01 21:28 UTC (permalink / raw)
  To: davem, netdev; +Cc: olaf, jasowang, driverdev-devel, linux-kernel, haiyangz

This patch adds proper handling of the vNIC hot removal event, which includes
a rescind-channel-offer message from the host side that triggers vNIC close and
removal. In this case, the notices to the host during close and removal is not
necessary because the channel is rescinded. This patch blocks these unnecessary 
messages, and lets vNIC removal process complete normally.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c         |    2 ++
 drivers/net/hyperv/netvsc.c       |    3 +++
 drivers/net/hyperv/rndis_filter.c |    3 +++
 include/linux/hyperv.h            |    2 ++
 4 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index a2d1a96..191a6a3 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -517,6 +517,8 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
 		/* Just return here, no channel found */
 		return;
 
+	channel->rescind = true;
+
 	/* work is initialized for vmbus_process_rescind_offer() from
 	 * vmbus_process_offer() where the channel got created */
 	queue_work(channel->controlwq, &channel->work);
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 6fc834e..dd867e6 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -764,6 +764,9 @@ int netvsc_send(struct hv_device *device,
 		out_channel = device->channel;
 	packet->channel = out_channel;
 
+	if (out_channel->rescind)
+		return -ENODEV;
+
 	if (packet->page_buf_cnt) {
 		ret = vmbus_sendpacket_pagebuffer(out_channel,
 						  packet->page_buf,
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 7b2c5d1..ec0c40a 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -958,6 +958,9 @@ static int rndis_filter_close_device(struct rndis_device *dev)
 		return 0;
 
 	ret = rndis_filter_set_packet_filter(dev, 0);
+	if (ret == -ENODEV)
+		ret = 0;
+
 	if (ret == 0)
 		dev->state = RNDIS_DEV_INITIALIZED;
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 08cfaff..476c685 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -650,6 +650,8 @@ struct vmbus_channel {
 	u8 monitor_grp;
 	u8 monitor_bit;
 
+	bool rescind; /* got rescind msg */
+
 	u32 ringbuffer_gpadlhandle;
 
 	/* Allocated memory for ring buffer */
-- 
1.7.1

^ permalink raw reply related

* Re: Is this 32-bit NCM?
From: Eli Britstein @ 2014-12-01 22:02 UTC (permalink / raw)
  To: Enrico Mioso
  Cc: Kevin Zhu, Bjørn Mork, Alex Strizhevsky, Midge Shaojun Tan,
	youtux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <alpine.LNX.2.03.1412012202480.1301-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi Enrico and all,

Maybe I missed something but what is the difference by the driver point of view between the dhcp discover and request (that are ok) to others (like arp, that is nok)?
Maybe we can trace to compare them?

Sent from my iPhone

> On 1 בדצמ 2014, at 23:11, "Enrico Mioso" <mrkiko.rs@gmail.com> wrote:
>
> So ... I have two ideas left for now.
> We know for sure the problem is in the way we TX frames, not the way we RX them
> (the way we send, generate them, not the way we receive them).
> Si I have two ideas, and I ask for help from the Linux-usb mailing list for
> this first one.
>
> 1 - Does a wayexist to "replay" traffic crom a usb capture?
> We might try to take the usb frames generated by Windows, and send them to the
> device to see if there is any reaction. It should not be science fiction, I saw
> them do that in the eciadsl old project.
> 2 - The huawei ndis driver: does it work with these devices?
> It should be a little bit out-dated now (at least in terms of dates, it might
> work as well): the code is A LOT but, just in case, to see if there is any
> chances it'll work. It remains to be seen in which kernels it can actually
> compile again.
>
> If this works we might analyse what's happening and try to debug this out.
> But I really would like this to work in the cdc_ncm driver + huawei_cdc_ncm.
> Thank you.

________________________________

This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.

If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message

^ permalink raw reply

* Re: BCM4313 & brcmsmac & 3.12: only semi-working?
From: Maximilian Engelhardt @ 2014-12-01 22:06 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: Michael Tokarev, Rafał Miłecki, Seth Forshee,
	brcm80211 development, linux-wireless@vger.kernel.org,
	Network Development
In-Reply-To: <5475A2DC.80502@msgid.tls.msk.ru>

[-- Attachment #1: Type: text/plain, Size: 10527 bytes --]

Hi Arend,

sorry for my late reply, it took me a bit to get the modules compiled with 
your patches. Here is the information from my card. During this test I had 
very bad performance with my wifi connection with big latency and lots of 
packet loss.

[ 1642.407735] bcma: bus0: Found chip with id 0x4313, rev 0x01 and package 0x08
[ 1642.407766] bcma: bus0: Core 0 found: ChipCommon (manuf 0x4BF, id 0x800, rev 0x24, class 0x0)
[ 1642.407794] bcma: bus0: Core 1 found: IEEE 802.11 (manuf 0x4BF, id 0x812, rev 0x18, class 0x0)
[ 1642.407829] bcma: bus0: Core 2 found: PCIe (manuf 0x4BF, id 0x820, rev 0x11, class 0x0)
[ 1642.407913] bcma: bus0: Found rev 8 PMU (capabilities 0x084C3008)
[ 1642.407978] bcma: bus0: SPROM offset 0x830
[ 1642.410625] bcma: bcmasprom:
[ 1642.410630] bcma: 
        
[ 1642.410635] bcma: 2801 
[ 1642.410637] bcma: 0000 
[ 1642.410640] bcma: 0608 
[ 1642.410642] bcma: 14E4 
[ 1642.410645] bcma: 0070 
[ 1642.410647] bcma: EDBE 
[ 1642.410649] bcma: 0000 
[ 1642.410651] bcma: 2BC4 
[ 1642.410654] bcma: 2A64 
[ 1642.410655] bcma: 2964 
[ 1642.410658] bcma: 
        
[ 1642.410659] bcma: 2C64 
[ 1642.410662] bcma: 3CE7 
[ 1642.410663] bcma: 46FF 
[ 1642.410666] bcma: 47FF 
[ 1642.410667] bcma: 0C00 
[ 1642.410670] bcma: 0820 
[ 1642.410671] bcma: 0030 
[ 1642.410674] bcma: 1002 
[ 1642.410676] bcma: 9F28 
[ 1642.410678] bcma: 5D44 
[ 1642.410679] bcma: 
        
[ 1642.410683] bcma: 8080 
[ 1642.410684] bcma: 1D8F 
[ 1642.410687] bcma: 0032 
[ 1642.410689] bcma: 0100 
[ 1642.410691] bcma: DF00 
[ 1642.410693] bcma: 71F5 
[ 1642.410695] bcma: 8400 
[ 1642.410697] bcma: 0083 
[ 1642.410699] bcma: 8500 
[ 1642.410701] bcma: 2010 
[ 1642.410703] bcma: 
        
[ 1642.410705] bcma: 0001 
[ 1642.410707] bcma: 0000 
[ 1642.410709] bcma: 0000 
[ 1642.410711] bcma: 0000 
[ 1642.410713] bcma: 0000 
[ 1642.410715] bcma: 0000 
[ 1642.410717] bcma: 0000 
[ 1642.410719] bcma: 0000 
[ 1642.410721] bcma: 0000 
[ 1642.410723] bcma: 0000 
[ 1642.410724] bcma: 
        
[ 1642.410728] bcma: 0000 
[ 1642.410730] bcma: 0000 
[ 1642.410732] bcma: 1008 
[ 1642.410734] bcma: 0305 
[ 1642.410736] bcma: 0000 
[ 1642.410738] bcma: 0000 
[ 1642.410740] bcma: 0000 
[ 1642.410742] bcma: 0000 
[ 1642.410744] bcma: 4727 
[ 1642.410746] bcma: 8000 
[ 1642.410748] bcma: 
        
[ 1642.410749] bcma: 0002 
[ 1642.410752] bcma: 0000 
[ 1642.410753] bcma: 1F30 
[ 1642.410756] bcma: 1800 
[ 1642.410757] bcma: 0000 
[ 1642.410760] bcma: 0000 
[ 1642.410761] bcma: 0000 
[ 1642.410764] bcma: 0000 
[ 1642.410765] bcma: 0000 
[ 1642.410768] bcma: 0000 
[ 1642.410769] bcma: 
        
[ 1642.410773] bcma: 0000 
[ 1642.410774] bcma: 0000 
[ 1642.410777] bcma: 0000 
[ 1642.410778] bcma: 0000 
[ 1642.410781] bcma: 5372 
[ 1642.410782] bcma: 1109 
[ 1642.410785] bcma: 2201 
[ 1642.410786] bcma: 0040 
[ 1642.410789] bcma: 0884 
[ 1642.410790] bcma: 0000 
[ 1642.410793] bcma: 
        
[ 1642.410794] bcma: C014 
[ 1642.410797] bcma: 3DC1 
[ 1642.410798] bcma: 809D 
[ 1642.410801] bcma: 5856 
[ 1642.410802] bcma: 0001 
[ 1642.410805] bcma: FFFF 
[ 1642.410807] bcma: 83FF 
[ 1642.410809] bcma: FFFF 
[ 1642.410811] bcma: 0003 
[ 1642.410813] bcma: 0202 
[ 1642.410814] bcma: 
        
[ 1642.410818] bcma: FFFF 
[ 1642.410819] bcma: 0011 
[ 1642.410822] bcma: 017A 
[ 1642.410824] bcma: 0000 
[ 1642.410826] bcma: 0000 
[ 1642.410828] bcma: 0000 
[ 1642.410830] bcma: 0000 
[ 1642.410832] bcma: 0201 
[ 1642.410834] bcma: 0000 
[ 1642.410836] bcma: 7800 
[ 1642.410838] bcma: 
        
[ 1642.410839] bcma: 01FF 
[ 1642.410842] bcma: E398 
[ 1642.410844] bcma: 0008 
[ 1642.410846] bcma: 0000 
[ 1642.410848] bcma: 0000 
[ 1642.410850] bcma: 0000 
[ 1642.410852] bcma: 0044 
[ 1642.410854] bcma: 2400 
[ 1642.410856] bcma: FCF7 
[ 1642.410858] bcma: 0089 
[ 1642.410860] bcma: 
        
[ 1642.410863] bcma: 0000 
[ 1642.410865] bcma: 0000 
[ 1642.410867] bcma: 0000 
[ 1642.410869] bcma: 0000 
[ 1642.410871] bcma: 0000 
[ 1642.410873] bcma: 0000 
[ 1642.410875] bcma: 0000 
[ 1642.410877] bcma: 0000 
[ 1642.410879] bcma: 0000 
[ 1642.410881] bcma: 0000 
[ 1642.410883] bcma: 
        
[ 1642.410884] bcma: 0000 
[ 1642.410887] bcma: 0000 
[ 1642.410888] bcma: 0048 
[ 1642.410891] bcma: FED2 
[ 1642.410892] bcma: 15D9 
[ 1642.410895] bcma: FAC6 
[ 1642.410897] bcma: 0000 
[ 1642.410899] bcma: 0000 
[ 1642.410901] bcma: 0000 
[ 1642.410903] bcma: 0000 
[ 1642.410904] bcma: 
        
[ 1642.410908] bcma: 0000 
[ 1642.410909] bcma: 0000 
[ 1642.410912] bcma: 0000 
[ 1642.410913] bcma: 0000 
[ 1642.410916] bcma: 0000 
[ 1642.410917] bcma: 0000 
[ 1642.410920] bcma: 0000 
[ 1642.410921] bcma: 0000 
[ 1642.410924] bcma: 0000 
[ 1642.410925] bcma: 0000 
[ 1642.410927] bcma: 
        
[ 1642.410929] bcma: 0000 
[ 1642.410931] bcma: 0000 
[ 1642.410933] bcma: 0000 
[ 1642.410935] bcma: 0000 
[ 1642.410937] bcma: 0000 
[ 1642.410939] bcma: 0000 
[ 1642.410941] bcma: 0000 
[ 1642.410943] bcma: 0000 
[ 1642.410945] bcma: 0000 
[ 1642.410947] bcma: 0000 
[ 1642.410949] bcma: 
        
[ 1642.410952] bcma: 0000 
[ 1642.410954] bcma: 0000 
[ 1642.410956] bcma: 0000 
[ 1642.410958] bcma: 0000 
[ 1642.410960] bcma: 0000 
[ 1642.410962] bcma: 0000 
[ 1642.410964] bcma: 0000 
[ 1642.410966] bcma: 0000 
[ 1642.410968] bcma: 0000 
[ 1642.410970] bcma: 0000 
[ 1642.410972] bcma: 
        
[ 1642.410974] bcma: 0000 
[ 1642.410976] bcma: 0000 
[ 1642.410978] bcma: 0000 
[ 1642.410980] bcma: 0000 
[ 1642.410982] bcma: 0000 
[ 1642.410984] bcma: 0000 
[ 1642.410986] bcma: 0000 
[ 1642.410988] bcma: 0000 
[ 1642.410990] bcma: 0000 
[ 1642.410992] bcma: 0000 
[ 1642.410994] bcma: 
        
[ 1642.410997] bcma: 0000 
[ 1642.410999] bcma: 1111 
[ 1642.411001] bcma: 1111 
[ 1642.411003] bcma: 0000 
[ 1642.411005] bcma: 0000 
[ 1642.411007] bcma: 0000 
[ 1642.411009] bcma: 0000 
[ 1642.411010] bcma: 0000 
[ 1642.411013] bcma: 0000 
[ 1642.411015] bcma: 2222 
[ 1642.411017] bcma: 
        
[ 1642.411018] bcma: 3222 
[ 1642.411021] bcma: 0000 
[ 1642.411022] bcma: 0000 
[ 1642.411025] bcma: 0000 
[ 1642.411026] bcma: 0000 
[ 1642.411029] bcma: 0000 
[ 1642.411030] bcma: 0000 
[ 1642.411033] bcma: 0000 
[ 1642.411034] bcma: 0000 
[ 1642.411037] bcma: 0000 
[ 1642.411038] bcma: 
        
[ 1642.411041] bcma: 0000 
[ 1642.411043] bcma: 0000 
[ 1642.411045] bcma: 0000 
[ 1642.411047] bcma: 0000 
[ 1642.411050] bcma: 0000 
[ 1642.411051] bcma: 0000 
[ 1642.411054] bcma: 0000 
[ 1642.411055] bcma: 0000 
[ 1642.411058] bcma: 0000 
[ 1642.411059] bcma: 0000 
[ 1642.411061] bcma: 
        
[ 1642.411063] bcma: 0000 
[ 1642.411065] bcma: 0000 
[ 1642.411067] bcma: 0000 
[ 1642.411069] bcma: 0000 
[ 1642.411071] bcma: 0000 
[ 1642.411073] bcma: 0000 
[ 1642.411075] bcma: 0000 
[ 1642.411077] bcma: 0000 
[ 1642.411079] bcma: 0000 
[ 1642.411081] bcma: 0000 
[ 1642.411083] bcma: 
        
[ 1642.411086] bcma: 0000 
[ 1642.411088] bcma: 0000 
[ 1642.411090] bcma: 0000 
[ 1642.411092] bcma: 0000 
[ 1642.411094] bcma: 0000 
[ 1642.411096] bcma: 0000 
[ 1642.411098] bcma: 0000 
[ 1642.411100] bcma: 0000 
[ 1642.411102] bcma: 0000 
[ 1642.411104] bcma: 0000 
[ 1642.411106] bcma: 
        
[ 1642.411107] bcma: 0000 
[ 1642.411110] bcma: 0000 
[ 1642.411111] bcma: 0000 
[ 1642.411114] bcma: 0000 
[ 1642.411115] bcma: 0000 
[ 1642.411118] bcma: 0000 
[ 1642.411119] bcma: 0000 
[ 1642.411122] bcma: 0000 
[ 1642.411123] bcma: 0000 
[ 1642.411127] bcma: bus0: Found SPROM revision 8
[ 1642.434510] bcma: bus0: GPIO driver not activated
[ 1642.434518] bcma: bus0: Bus registered
[ 1642.449971] brcmsmac bcma0:0: mfg 4bf core 812 rev 24 class 0 irq 17
[ 1642.450095] bcma: bus0: Switched to core: 0x812
[ 1642.451446] ieee80211 phy1: Selected rate control algorithm 'minstrel_ht'




[ 4112.897927] brcmsmac bcma0:0: brcms_ops_bss_info_changed: qos enabled: false (implement)
[ 4112.898113] brcmsmac bcma0:0: brcms_ops_config: change power-save mode: false (implement)
[ 4112.899832] IPv6: ADDRCONF(NETDEV_UP): wlan0: link is not ready
[ 4113.830658] wlan0: authenticate with e8:de:27:44:3b:33
[ 4113.830764] wlan0: send auth to e8:de:27:44:3b:33 (try 1/3)
[ 4113.832620] wlan0: authenticated
[ 4113.833859] wlan0: associate with e8:de:27:44:3b:33 (try 1/3)
[ 4113.837231] wlan0: RX AssocResp from e8:de:27:44:3b:33 (capab=0x431 status=0 aid=1)
[ 4113.837591] brcmsmac bcma0:0: brcmsmac: brcms_ops_bss_info_changed: associated
[ 4113.837752] brcmsmac bcma0:0: brcms_ops_bss_info_changed: arp filtering: 1 addresses (implement)
[ 4113.837861] brcmsmac bcma0:0: brcms_ops_bss_info_changed: qos enabled: true (implement)
[ 4113.838209] wlan0: associated
[ 4113.838228] IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
[ 4113.838771] cfg80211: Calling CRDA for country: DE
[ 4113.847285] cfg80211: Regulatory domain changed to country: DE
[ 4113.847298] cfg80211:  DFS Master region: ETSI
[ 4113.847302] cfg80211:   (start_freq - end_freq @ bandwidth), (max_antenna_gain, max_eirp), (dfs_cac_time)
[ 4113.847308] cfg80211:   (2400000 KHz - 2483500 KHz @ 40000 KHz), (N/A, 2000 mBm), (N/A)
[ 4113.847313] cfg80211:   (5150000 KHz - 5250000 KHz @ 80000 KHz), (N/A, 2000 mBm), (N/A)
[ 4113.847318] cfg80211:   (5250000 KHz - 5350000 KHz @ 80000 KHz), (N/A, 2000 mBm), (0 s)
[ 4113.847322] cfg80211:   (5470000 KHz - 5725000 KHz @ 80000 KHz), (N/A, 2698 mBm), (0 s)
[ 4113.847327] cfg80211:   (57240000 KHz - 65880000 KHz @ 2160000 KHz), (N/A, 4000 mBm), (N/A)



$ cat brcmsmac/bcma0\:0/hardware 
chipnum 0x4313
chiprev 0x1
chippackage 0x8
corerev 0x18
boardid 0x608
boardvendor 0x14e4
boardrev P109
boardflags 0x402201
boardflags2 0x884
ucoderev 0x262032c
radiorev 0x1
phytype 0x8
phyrev 0x1
anarev 0xa
nvramrev 8


$ cat brcmsmac/bcma0\:0/macstat 
txallfrm: 1622
txrtsfrm: 739
txctsfrm: 269
txackfrm: 128
txdnlfrm: 0
txbcnfrm: 0
txfunfl[8]: 0 0 0 0 0 0 0 0
txtplunfl: 0
txphyerr: 0
pktengrxducast: 0
pktengrxdmcast: 0
rxfrmtoolong: 927
rxfrmtooshrt: 158
rxinvmachdr: 3145
rxbadfcs: 7459
rxbadplcp: 28631
rxcrsglitch: 945
rxstrt: 17138
rxdfrmucastmbss: 112
rxmfrmucastmbss: 16
rxcfrmucast: 745
rxrtsucast: 269
rxctsucast: 311
rxackucast: 38
rxdfrmocast: 21
rxmfrmocast: 16
rxcfrmocast: 7664
rxrtsocast: 379
rxctsocast: 6351
rxdfrmmcast: 5
rxmfrmmcast: 141
rxcfrmmcast: 0
rxbeaconmbss: 47
rxdfrmucastobss: 0
rxbeaconobss: 55
rxrsptmout: 650
bcntxcancl: 0
rxf0ovfl: 0
rxf1ovfl: 0
rxf2ovfl: 0
txsfovfl: 0
pmqovfl: 0
rxcgprqfrm: 0
rxcgprsqovfl: 0
txcgprsfail: 0
txcgprssuc: 0
prs_timeout: 0
rxnack: 0
frmscons: 0
txnack: 0
txglitch_nack: 127
txburst: 0
bphy_rxcrsglitch: 1
phywatchdog: 0
bphy_badplcp: 0

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH v2 net-next] net: bcmgenet: enable driver to work without a device tree
From: Petri Gynther @ 2014-12-01 22:08 UTC (permalink / raw)
  To: netdev; +Cc: davem, f.fainelli

Broadcom 7xxx MIPS-based STB platforms do not use device trees.
Modify bcmgenet driver so that it can be used on those platforms.

Signed-off-by: Petri Gynther <pgynther@google.com>
---
 drivers/net/ethernet/broadcom/Kconfig          |   1 -
 drivers/net/ethernet/broadcom/genet/bcmgenet.c |  31 ++++--
 drivers/net/ethernet/broadcom/genet/bcmmii.c   | 134 +++++++++++++++++++++----
 include/linux/platform_data/bcmgenet.h         |  18 ++++
 4 files changed, 152 insertions(+), 32 deletions(-)
 create mode 100644 include/linux/platform_data/bcmgenet.h

diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index c3e260c..888247a 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -62,7 +62,6 @@ config BCM63XX_ENET
 
 config BCMGENET
 	tristate "Broadcom GENET internal MAC support"
-	depends on OF
 	select MII
 	select PHYLIB
 	select FIXED_PHY if BCMGENET=y
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index f2fadb0..861c298 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -42,6 +42,7 @@
 #include <linux/ip.h>
 #include <linux/ipv6.h>
 #include <linux/phy.h>
+#include <linux/platform_data/bcmgenet.h>
 
 #include <asm/unaligned.h>
 
@@ -2586,6 +2587,7 @@ static const struct of_device_id bcmgenet_match[] = {
 
 static int bcmgenet_probe(struct platform_device *pdev)
 {
+	struct bcmgenet_platform_data *pd = pdev->dev.platform_data;
 	struct device_node *dn = pdev->dev.of_node;
 	const struct of_device_id *of_id;
 	struct bcmgenet_priv *priv;
@@ -2601,9 +2603,13 @@ static int bcmgenet_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	of_id = of_match_node(bcmgenet_match, dn);
-	if (!of_id)
-		return -EINVAL;
+	if (dn) {
+		of_id = of_match_node(bcmgenet_match, dn);
+		if (!of_id)
+			return -EINVAL;
+	} else {
+		of_id = NULL;
+	}
 
 	priv = netdev_priv(dev);
 	priv->irq0 = platform_get_irq(pdev, 0);
@@ -2615,11 +2621,15 @@ static int bcmgenet_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	macaddr = of_get_mac_address(dn);
-	if (!macaddr) {
-		dev_err(&pdev->dev, "can't find MAC address\n");
-		err = -EINVAL;
-		goto err;
+	if (dn) {
+		macaddr = of_get_mac_address(dn);
+		if (!macaddr) {
+			dev_err(&pdev->dev, "can't find MAC address\n");
+			err = -EINVAL;
+			goto err;
+		}
+	} else {
+		macaddr = pd->macaddr;
 	}
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -2659,7 +2669,10 @@ static int bcmgenet_probe(struct platform_device *pdev)
 
 	priv->dev = dev;
 	priv->pdev = pdev;
-	priv->version = (enum bcmgenet_version)of_id->data;
+	if (of_id)
+		priv->version = (enum bcmgenet_version)of_id->data;
+	else
+		priv->version = pd->genet_version;
 
 	priv->clk = devm_clk_get(&priv->pdev->dev, "enet");
 	if (IS_ERR(priv->clk))
diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 933cd7e..f758686 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -23,6 +23,7 @@
 #include <linux/of.h>
 #include <linux/of_net.h>
 #include <linux/of_mdio.h>
+#include <linux/platform_data/bcmgenet.h>
 
 #include "bcmgenet.h"
 
@@ -312,22 +313,6 @@ static int bcmgenet_mii_probe(struct net_device *dev)
 	u32 phy_flags;
 	int ret;
 
-	if (priv->phydev) {
-		pr_info("PHY already attached\n");
-		return 0;
-	}
-
-	/* In the case of a fixed PHY, the DT node associated
-	 * to the PHY is the Ethernet MAC DT node.
-	 */
-	if (!priv->phy_dn && of_phy_is_fixed_link(dn)) {
-		ret = of_phy_register_fixed_link(dn);
-		if (ret)
-			return ret;
-
-		priv->phy_dn = of_node_get(dn);
-	}
-
 	/* Communicate the integrated PHY revision */
 	phy_flags = priv->gphy_rev;
 
@@ -337,11 +322,39 @@ static int bcmgenet_mii_probe(struct net_device *dev)
 	priv->old_duplex = -1;
 	priv->old_pause = -1;
 
-	phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup,
-				phy_flags, priv->phy_interface);
-	if (!phydev) {
-		pr_err("could not attach to PHY\n");
-		return -ENODEV;
+	if (dn) {
+		if (priv->phydev) {
+			pr_info("PHY already attached\n");
+			return 0;
+		}
+
+		/* In the case of a fixed PHY, the DT node associated
+		 * to the PHY is the Ethernet MAC DT node.
+		 */
+		if (!priv->phy_dn && of_phy_is_fixed_link(dn)) {
+			ret = of_phy_register_fixed_link(dn);
+			if (ret)
+				return ret;
+
+			priv->phy_dn = of_node_get(dn);
+		}
+
+		phydev = of_phy_connect(dev, priv->phy_dn, bcmgenet_mii_setup,
+					phy_flags, priv->phy_interface);
+		if (!phydev) {
+			pr_err("could not attach to PHY\n");
+			return -ENODEV;
+		}
+	} else {
+		phydev = priv->phydev;
+		phydev->dev_flags = phy_flags;
+
+		ret = phy_connect_direct(dev, phydev, bcmgenet_mii_setup,
+					 priv->phy_interface);
+		if (ret) {
+			pr_err("could not attach to PHY\n");
+			return -ENODEV;
+		}
 	}
 
 	priv->phydev = phydev;
@@ -438,6 +451,83 @@ static int bcmgenet_mii_of_init(struct bcmgenet_priv *priv)
 	return 0;
 }
 
+static int bcmgenet_mii_pd_init(struct bcmgenet_priv *priv)
+{
+	struct device *kdev = &priv->pdev->dev;
+	struct bcmgenet_platform_data *pd = kdev->platform_data;
+	struct mii_bus *mdio = priv->mii_bus;
+	struct phy_device *phydev;
+	int ret;
+	int i;
+
+	if (pd->phy_type != PHY_INTERFACE_MODE_MOCA && pd->mdio_enabled) {
+		/*
+		 * Internal or external PHY with MDIO access
+		 */
+		if (pd->phy_addr >= 0 && pd->phy_addr < PHY_MAX_ADDR)
+			mdio->phy_mask = ~(1 << pd->phy_addr);
+		else
+			mdio->phy_mask = 0;
+
+		ret = mdiobus_register(mdio);
+		if (ret) {
+			dev_err(kdev, "failed to register MDIO bus\n");
+			return ret;
+		}
+
+		if (pd->phy_addr >= 0 && pd->phy_addr < PHY_MAX_ADDR) {
+			phydev = mdio->phy_map[pd->phy_addr];
+		} else {
+			phydev = NULL;
+			for (i = 0; i < PHY_MAX_ADDR; i++) {
+				if (mdio->phy_map[i]) {
+					phydev = mdio->phy_map[i];
+					break;
+				}
+			}
+		}
+
+		if (!phydev) {
+			dev_err(kdev, "failed to register PHY device\n");
+			mdiobus_unregister(mdio);
+			return -ENODEV;
+		}
+	} else {
+		/*
+		 * MoCA port or no MDIO access.
+		 * Use 1000/FD fixed PHY to represent the link layer.
+		 */
+		struct fixed_phy_status fphy_status = {
+			.link = 1,
+			.duplex = DUPLEX_FULL,
+			.speed = SPEED_1000,
+			.pause = 0,
+			.asym_pause = 0,
+		};
+
+		phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
+		if (!phydev || IS_ERR(phydev)) {
+			dev_err(kdev, "failed to register fixed PHY device\n");
+			return -ENODEV;
+		}
+	}
+
+	priv->phydev = phydev;
+	priv->phy_interface = pd->phy_type;
+
+	return 0;
+}
+
+static int bcmgenet_mii_bus_init(struct bcmgenet_priv *priv)
+{
+	struct device_node *dn = priv->pdev->dev.of_node;
+
+	if (dn)
+		return bcmgenet_mii_of_init(priv);
+	else
+		return bcmgenet_mii_pd_init(priv);
+}
+
 int bcmgenet_mii_init(struct net_device *dev)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
@@ -447,7 +537,7 @@ int bcmgenet_mii_init(struct net_device *dev)
 	if (ret)
 		return ret;
 
-	ret = bcmgenet_mii_of_init(priv);
+	ret = bcmgenet_mii_bus_init(priv);
 	if (ret)
 		goto out_free;
 
diff --git a/include/linux/platform_data/bcmgenet.h b/include/linux/platform_data/bcmgenet.h
new file mode 100644
index 0000000..3660133
--- /dev/null
+++ b/include/linux/platform_data/bcmgenet.h
@@ -0,0 +1,18 @@
+#ifndef __LINUX_PLATFORM_DATA_BCMGENET_H__
+#define __LINUX_PLATFORM_DATA_BCMGENET_H__
+
+#include <linux/compiler.h>
+#include <linux/if_ether.h>
+
+struct bcmgenet_platform_data {
+	void __iomem *base_reg;
+	int           irq0;
+	int           irq1;
+	bool          mdio_enabled;
+	int           phy_type;
+	int           phy_addr;
+	u8            macaddr[ETH_ALEN];
+	int           genet_version;
+};
+
+#endif
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related

* [PATCH net-next 0/4] openvswitch: MPLS related fixes.
From: Pravin B Shelar @ 2014-12-01 22:30 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

First three patches fixes and improves OVS MPLS support. Last
patch is general code cleanup.

Pravin B Shelar (4):
  openvswitch: Set inner-most protocol for MPLS.
  vlan: Fix mac_len adjustment.
  mpls: Fix allowed protocols for mpls gso
  openvswitch: Fix coding style.

 net/core/skbuff.c              |    2 +-
 net/mpls/mpls_gso.c            |    5 +----
 net/openvswitch/actions.c      |    6 +++---
 net/openvswitch/datapath.c     |   11 ++++++-----
 net/openvswitch/flow.c         |   11 ++++++-----
 net/openvswitch/flow_netlink.c |   12 +++++++-----
 net/openvswitch/flow_table.c   |    3 ++-
 7 files changed, 26 insertions(+), 24 deletions(-)

^ permalink raw reply

* [PATCH net-next 1/4] openvswitch: Set inner-most protocol for MPLS.
From: Pravin B Shelar @ 2014-12-01 22:30 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

MPLS GSO needs to know inner most protocol to process GSO packets.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/openvswitch/actions.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 764fdc3..770064c 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -147,7 +147,8 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
 	hdr = eth_hdr(skb);
 	hdr->h_proto = mpls->mpls_ethertype;
 
-	skb_set_inner_protocol(skb, skb->protocol);
+	if (!skb->inner_protocol)
+		skb_set_inner_protocol(skb, skb->protocol);
 	skb->protocol = mpls->mpls_ethertype;
 
 	invalidate_flow_key(key);
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 3/4] mpls: Fix allowed protocols for mpls gso
From: Pravin B Shelar @ 2014-12-01 22:30 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

MPLS and Tunnel GSO does not work together.  Reject packet which
request such GSO.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/mpls/mpls_gso.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/net/mpls/mpls_gso.c b/net/mpls/mpls_gso.c
index ca27837..349295d 100644
--- a/net/mpls/mpls_gso.c
+++ b/net/mpls/mpls_gso.c
@@ -31,10 +31,7 @@ static struct sk_buff *mpls_gso_segment(struct sk_buff *skb,
 				  SKB_GSO_TCPV6 |
 				  SKB_GSO_UDP |
 				  SKB_GSO_DODGY |
-				  SKB_GSO_TCP_ECN |
-				  SKB_GSO_GRE |
-				  SKB_GSO_GRE_CSUM |
-				  SKB_GSO_IPIP)))
+				  SKB_GSO_TCP_ECN)))
 		goto out;
 
 	/* Setup inner SKB. */
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 2/4] vlan: Fix mac_len adjustment.
From: Pravin B Shelar @ 2014-12-01 22:30 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

skb_reset_mac_len() sets length according to ethernet and network
offsets, but mpls expects mac-length to be offset to mpls header (ref.
skb_mpls_header()). Therefore rather than reset we need to subtract
VLAN_HLEN from mac_len.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/core/skbuff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 92116df..c45888f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4178,7 +4178,7 @@ static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
 	if (skb_network_offset(skb) < ETH_HLEN)
 		skb_set_network_header(skb, ETH_HLEN);
 
-	skb_reset_mac_len(skb);
+	skb->mac_len -= VLAN_HLEN;
 pull:
 	__skb_pull(skb, offset);
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next 4/4] openvswitch: Fix coding style.
From: Pravin B Shelar @ 2014-12-01 22:30 UTC (permalink / raw)
  To: davem; +Cc: netdev, Pravin B Shelar

Fixes comment style issues, removes redundant egress_tun_info
reset.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 net/openvswitch/actions.c      |    3 +--
 net/openvswitch/datapath.c     |   11 ++++++-----
 net/openvswitch/flow.c         |   11 ++++++-----
 net/openvswitch/flow_netlink.c |   12 +++++++-----
 net/openvswitch/flow_table.c   |    3 ++-
 5 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 770064c..453d5b8 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -90,7 +90,7 @@ static struct deferred_action *action_fifo_put(struct action_fifo *fifo)
 	return &fifo->fifo[fifo->head++];
 }
 
-/* Return true if fifo is not full */
+/* Return queue entry if fifo is not full */
 static struct deferred_action *add_deferred_actions(struct sk_buff *skb,
 						    const struct sw_flow_key *key,
 						    const struct nlattr *attr)
@@ -872,7 +872,6 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
 	int err;
 
 	this_cpu_inc(exec_actions_level);
-	OVS_CB(skb)->egress_tun_info = NULL;
 	err = do_execute_actions(dp, skb, key,
 				 acts->actions, acts->actions_len);
 
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 332b5a0..55ce8c8 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -78,7 +78,8 @@ static const struct genl_multicast_group ovs_dp_vport_multicast_group = {
 };
 
 /* Check if need to build a reply message.
- * OVS userspace sets the NLM_F_ECHO flag if it needs the reply. */
+ * OVS userspace sets the NLM_F_ECHO flag if it needs the reply.
+ */
 static bool ovs_must_notify(struct genl_family *family, struct genl_info *info,
 			    unsigned int group)
 {
@@ -481,7 +482,8 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
 	}
 
 	/* Only reserve room for attribute header, packet data is added
-	 * in skb_zerocopy() */
+	 * in skb_zerocopy()
+	 */
 	if (!(nla = nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, 0))) {
 		err = -ENOBUFS;
 		goto out;
@@ -546,7 +548,8 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
 
 	/* Normally, setting the skb 'protocol' field would be handled by a
 	 * call to eth_type_trans(), but it assumes there's a sending
-	 * device, which we may not have. */
+	 * device, which we may not have.
+	 */
 	if (ntohs(eth->h_proto) >= ETH_P_802_3_MIN)
 		packet->protocol = eth->h_proto;
 	else
@@ -1894,7 +1897,6 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
 			goto exit_unlock_free;
 	}
 
-
 	if (a[OVS_VPORT_ATTR_UPCALL_PID]) {
 		struct nlattr *ids = a[OVS_VPORT_ATTR_UPCALL_PID];
 
@@ -1906,7 +1908,6 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
 	err = ovs_vport_cmd_fill_info(vport, reply, info->snd_portid,
 				      info->snd_seq, 0, OVS_VPORT_CMD_NEW);
 	BUG_ON(err < 0);
-
 	ovs_unlock();
 	ovs_notify(&dp_vport_genl_family, reply, info);
 	return 0;
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 70bef2a..96480fe 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -552,7 +552,8 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
 				struct icmphdr *icmp = icmp_hdr(skb);
 				/* The ICMP type and code fields use the 16-bit
 				 * transport port fields, so we need to store
-				 * them in 16-bit network byte order. */
+				 * them in 16-bit network byte order.
+				 */
 				key->tp.src = htons(icmp->type);
 				key->tp.dst = htons(icmp->code);
 			} else {
@@ -686,17 +687,17 @@ int ovs_flow_key_extract(const struct ovs_tunnel_info *tun_info,
 	if (tun_info) {
 		memcpy(&key->tun_key, &tun_info->tunnel, sizeof(key->tun_key));
 
+		BUILD_BUG_ON(((1 << (sizeof(tun_info->options_len) * 8)) - 1) >
+			     sizeof(key->tun_opts));
+
 		if (tun_info->options) {
-			BUILD_BUG_ON((1 << (sizeof(tun_info->options_len) *
-						   8)) - 1
-					> sizeof(key->tun_opts));
 			memcpy(GENEVE_OPTS(key, tun_info->options_len),
 			       tun_info->options, tun_info->options_len);
 			key->tun_opts_len = tun_info->options_len;
 		} else {
 			key->tun_opts_len = 0;
 		}
-	} else  {
+	} else {
 		key->tun_opts_len = 0;
 		memset(&key->tun_key, 0, sizeof(key->tun_key));
 	}
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index df3c7f2..3c92a86 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -18,8 +18,6 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include "flow.h"
-#include "datapath.h"
 #include <linux/uaccess.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
@@ -48,6 +46,8 @@
 #include <net/ndisc.h>
 #include <net/mpls.h>
 
+#include "datapath.h"
+#include "flow.h"
 #include "flow_netlink.h"
 
 static void update_range(struct sw_flow_match *match,
@@ -118,7 +118,8 @@ static bool match_validate(const struct sw_flow_match *match,
 	u64 mask_allowed = key_attrs;  /* At most allow all key attributes */
 
 	/* The following mask attributes allowed only if they
-	 * pass the validation tests. */
+	 * pass the validation tests.
+	 */
 	mask_allowed &= ~((1 << OVS_KEY_ATTR_IPV4)
 			| (1 << OVS_KEY_ATTR_IPV6)
 			| (1 << OVS_KEY_ATTR_TCP)
@@ -833,7 +834,7 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match, u64 attrs,
 				mpls_key->mpls_lse, is_mask);
 
 		attrs &= ~(1 << OVS_KEY_ATTR_MPLS);
-	 }
+	}
 
 	if (attrs & (1 << OVS_KEY_ATTR_TCP)) {
 		const struct ovs_key_tcp *tcp_key;
@@ -1366,7 +1367,8 @@ static struct sw_flow_actions *nla_alloc_flow_actions(int size, bool log)
 }
 
 /* Schedules 'sf_acts' to be freed after the next RCU grace period.
- * The caller must hold rcu_read_lock for this to be sensible. */
+ * The caller must hold rcu_read_lock for this to be sensible.
+ */
 void ovs_nla_free_flow_actions(struct sw_flow_actions *sf_acts)
 {
 	kfree_rcu(sf_acts, rcu);
diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index e0a7fef..d40fdb2 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -617,7 +617,8 @@ int ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
 }
 
 /* Initializes the flow module.
- * Returns zero if successful or a negative error code. */
+ * Returns zero if successful or a negative error code.
+ */
 int ovs_flow_init(void)
 {
 	BUILD_BUG_ON(__alignof__(struct sw_flow_key) % __alignof__(long));
-- 
1.7.1

^ permalink raw reply related

* [PATCH v2 net-next 0/6] allow eBPF programs to be attached to sockets
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel

V1->V2:

fixed comments in sample code to state clearly that packet data is accessed
with LD_ABS instructions and not internal skb fields.
Also replaced constants in:
BPF_LD_ABS(BPF_B, 14 + 9 /* R0 = ip->proto */),
with:
BPF_LD_ABS(BPF_B, ETH_HLEN + offsetof(struct iphdr, protocol) /* R0 = ip->proto */),

V1 cover:

Introduce BPF_PROG_TYPE_SOCKET_FILTER type of eBPF programs that can be
attached to sockets with setsockopt().
Allow such programs to access maps via lookup/update/delete helpers.

This feature was previewed by bpf manpage in commit b4fc1a460f30("Merge branch 'bpf-next'")
Now it can actually run.

1st patch adds LD_ABS/LD_IND instruction verification and
2nd patch adds new setsockopt() flag.
Patches 3-6 are examples in assembler and in C.

Though native eBPF programs are way more powerful than classic filters
(attachable through similar setsockopt() call), they don't have skb field
accessors yet. Like skb->pkt_type, skb->dev->ifindex are not accessible.
There are sevaral ways to achieve that. That will be in the next set of patches.
So in this set native eBPF programs can only read data from packet and
access maps.

The most powerful example is sockex2_kern.c from patch 6 where ~200 lines of C
are compiled into ~300 of eBPF instructions.
It shows how quite complex packet parsing can be done.

LLVM used to build examples is at https://github.com/iovisor/llvm
which is fork of llvm trunk that I'm cleaning up for upstreaming.

Alexei Starovoitov (6):
  bpf: verifier: add checks for BPF_ABS | BPF_IND instructions
  net: sock: allow eBPF programs to be attached to sockets
  samples: bpf: example of stateful socket filtering
  samples: bpf: elf_bpf file loader
  samples: bpf: trivial eBPF program in C
  samples: bpf: large eBPF program in C

 arch/alpha/include/uapi/asm/socket.h   |    3 +
 arch/avr32/include/uapi/asm/socket.h   |    3 +
 arch/cris/include/uapi/asm/socket.h    |    3 +
 arch/frv/include/uapi/asm/socket.h     |    3 +
 arch/ia64/include/uapi/asm/socket.h    |    3 +
 arch/m32r/include/uapi/asm/socket.h    |    3 +
 arch/mips/include/uapi/asm/socket.h    |    3 +
 arch/mn10300/include/uapi/asm/socket.h |    3 +
 arch/parisc/include/uapi/asm/socket.h  |    3 +
 arch/powerpc/include/uapi/asm/socket.h |    3 +
 arch/s390/include/uapi/asm/socket.h    |    3 +
 arch/sparc/include/uapi/asm/socket.h   |    3 +
 arch/xtensa/include/uapi/asm/socket.h  |    3 +
 include/linux/bpf.h                    |    4 +
 include/linux/filter.h                 |    1 +
 include/uapi/asm-generic/socket.h      |    3 +
 include/uapi/linux/bpf.h               |    1 +
 kernel/bpf/verifier.c                  |   70 ++++++++++-
 net/core/filter.c                      |   97 +++++++++++++-
 net/core/sock.c                        |   13 ++
 samples/bpf/Makefile                   |   20 +++
 samples/bpf/bpf_helpers.h              |   40 ++++++
 samples/bpf/bpf_load.c                 |  203 ++++++++++++++++++++++++++++++
 samples/bpf/bpf_load.h                 |   24 ++++
 samples/bpf/libbpf.c                   |   28 +++++
 samples/bpf/libbpf.h                   |   15 ++-
 samples/bpf/sock_example.c             |  101 +++++++++++++++
 samples/bpf/sockex1_kern.c             |   25 ++++
 samples/bpf/sockex1_user.c             |   49 ++++++++
 samples/bpf/sockex2_kern.c             |  215 ++++++++++++++++++++++++++++++++
 samples/bpf/sockex2_user.c             |   44 +++++++
 31 files changed, 987 insertions(+), 5 deletions(-)
 create mode 100644 samples/bpf/bpf_helpers.h
 create mode 100644 samples/bpf/bpf_load.c
 create mode 100644 samples/bpf/bpf_load.h
 create mode 100644 samples/bpf/sock_example.c
 create mode 100644 samples/bpf/sockex1_kern.c
 create mode 100644 samples/bpf/sockex1_user.c
 create mode 100644 samples/bpf/sockex2_kern.c
 create mode 100644 samples/bpf/sockex2_user.c

-- 
1.7.9.5

^ permalink raw reply

* [PATCH v2 net-next 1/6] bpf: verifier: add checks for BPF_ABS | BPF_IND instructions
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1417475199-15950-1-git-send-email-ast@plumgrid.com>

introduce program type BPF_PROG_TYPE_SOCKET_FILTER that is used
for attaching programs to sockets where ctx == skb.

add verifier checks for ABS/IND instructions which can only be seen
in socket filters, therefore the check:
  if (env->prog->aux->prog_type != BPF_PROG_TYPE_SOCKET_FILTER)
    verbose("BPF_LD_ABS|IND instructions are only allowed in socket filters\n");

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
v2: no changes

 include/uapi/linux/bpf.h |    1 +
 kernel/bpf/verifier.c    |   70 ++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 4a3d0f84f178..45da7ec7d274 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -117,6 +117,7 @@ enum bpf_map_type {
 
 enum bpf_prog_type {
 	BPF_PROG_TYPE_UNSPEC,
+	BPF_PROG_TYPE_SOCKET_FILTER,
 };
 
 /* flags for BPF_MAP_UPDATE_ELEM command */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b6a1f7c14a67..a28e09c7825d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1172,6 +1172,70 @@ static int check_ld_imm(struct verifier_env *env, struct bpf_insn *insn)
 	return 0;
 }
 
+/* verify safety of LD_ABS|LD_IND instructions:
+ * - they can only appear in the programs where ctx == skb
+ * - since they are wrappers of function calls, they scratch R1-R5 registers,
+ *   preserve R6-R9, and store return value into R0
+ *
+ * Implicit input:
+ *   ctx == skb == R6 == CTX
+ *
+ * Explicit input:
+ *   SRC == any register
+ *   IMM == 32-bit immediate
+ *
+ * Output:
+ *   R0 - 8/16/32-bit skb data converted to cpu endianness
+ */
+static int check_ld_abs(struct verifier_env *env, struct bpf_insn *insn)
+{
+	struct reg_state *regs = env->cur_state.regs;
+	u8 mode = BPF_MODE(insn->code);
+	struct reg_state *reg;
+	int i, err;
+
+	if (env->prog->aux->prog_type != BPF_PROG_TYPE_SOCKET_FILTER) {
+		verbose("BPF_LD_ABS|IND instructions are only allowed in socket filters\n");
+		return -EINVAL;
+	}
+
+	if (insn->dst_reg != BPF_REG_0 || insn->off != 0 ||
+	    (mode == BPF_ABS && insn->src_reg != BPF_REG_0)) {
+		verbose("BPF_LD_ABS uses reserved fields\n");
+		return -EINVAL;
+	}
+
+	/* check whether implicit source operand (register R6) is readable */
+	err = check_reg_arg(regs, BPF_REG_6, SRC_OP);
+	if (err)
+		return err;
+
+	if (regs[BPF_REG_6].type != PTR_TO_CTX) {
+		verbose("at the time of BPF_LD_ABS|IND R6 != pointer to skb\n");
+		return -EINVAL;
+	}
+
+	if (mode == BPF_IND) {
+		/* check explicit source operand */
+		err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+		if (err)
+			return err;
+	}
+
+	/* reset caller saved regs to unreadable */
+	for (i = 0; i < CALLER_SAVED_REGS; i++) {
+		reg = regs + caller_saved[i];
+		reg->type = NOT_INIT;
+		reg->imm = 0;
+	}
+
+	/* mark destination R0 register as readable, since it contains
+	 * the value fetched from the packet
+	 */
+	regs[BPF_REG_0].type = UNKNOWN_VALUE;
+	return 0;
+}
+
 /* non-recursive DFS pseudo code
  * 1  procedure DFS-iterative(G,v):
  * 2      label v as discovered
@@ -1677,8 +1741,10 @@ process_bpf_exit:
 			u8 mode = BPF_MODE(insn->code);
 
 			if (mode == BPF_ABS || mode == BPF_IND) {
-				verbose("LD_ABS is not supported yet\n");
-				return -EINVAL;
+				err = check_ld_abs(env, insn);
+				if (err)
+					return err;
+
 			} else if (mode == BPF_IMM) {
 				err = check_ld_imm(env, insn);
 				if (err)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 2/6] net: sock: allow eBPF programs to be attached to sockets
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet,
	linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1417475199-15950-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>

introduce new setsockopt() command:

setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd, sizeof(prog_fd))

where prog_fd was received from syscall bpf(BPF_PROG_LOAD, attr, ...)
and attr->prog_type == BPF_PROG_TYPE_SOCKET_FILTER

setsockopt() calls bpf_prog_get() which increments refcnt of the program,
so it doesn't get unloaded while socket is using the program.

The same eBPF program can be attached to multiple sockets.

User task exit automatically closes socket which calls sk_filter_uncharge()
which decrements refcnt of eBPF program

Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
v2: no changes

Note, I'm not happy about 'ifdef', but 'select or depend BPF_SYSCALL' will
make tinification folks cringe, so use ifdef until native eBPF use cases
become widespread.

 arch/alpha/include/uapi/asm/socket.h   |    3 +
 arch/avr32/include/uapi/asm/socket.h   |    3 +
 arch/cris/include/uapi/asm/socket.h    |    3 +
 arch/frv/include/uapi/asm/socket.h     |    3 +
 arch/ia64/include/uapi/asm/socket.h    |    3 +
 arch/m32r/include/uapi/asm/socket.h    |    3 +
 arch/mips/include/uapi/asm/socket.h    |    3 +
 arch/mn10300/include/uapi/asm/socket.h |    3 +
 arch/parisc/include/uapi/asm/socket.h  |    3 +
 arch/powerpc/include/uapi/asm/socket.h |    3 +
 arch/s390/include/uapi/asm/socket.h    |    3 +
 arch/sparc/include/uapi/asm/socket.h   |    3 +
 arch/xtensa/include/uapi/asm/socket.h  |    3 +
 include/linux/bpf.h                    |    4 ++
 include/linux/filter.h                 |    1 +
 include/uapi/asm-generic/socket.h      |    3 +
 net/core/filter.c                      |   97 +++++++++++++++++++++++++++++++-
 net/core/sock.c                        |   13 +++++
 18 files changed, 155 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index e2fe0700b3b4..9a20821b111c 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -89,4 +89,7 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/avr32/include/uapi/asm/socket.h b/arch/avr32/include/uapi/asm/socket.h
index 92121b0f5b98..2b65ed6b277c 100644
--- a/arch/avr32/include/uapi/asm/socket.h
+++ b/arch/avr32/include/uapi/asm/socket.h
@@ -82,4 +82,7 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* _UAPI__ASM_AVR32_SOCKET_H */
diff --git a/arch/cris/include/uapi/asm/socket.h b/arch/cris/include/uapi/asm/socket.h
index 60f60f5b9b35..e2503d9f1869 100644
--- a/arch/cris/include/uapi/asm/socket.h
+++ b/arch/cris/include/uapi/asm/socket.h
@@ -84,6 +84,9 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* _ASM_SOCKET_H */
 
 
diff --git a/arch/frv/include/uapi/asm/socket.h b/arch/frv/include/uapi/asm/socket.h
index 2c6890209ea6..4823ad125578 100644
--- a/arch/frv/include/uapi/asm/socket.h
+++ b/arch/frv/include/uapi/asm/socket.h
@@ -82,5 +82,8 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* _ASM_SOCKET_H */
 
diff --git a/arch/ia64/include/uapi/asm/socket.h b/arch/ia64/include/uapi/asm/socket.h
index 09a93fb566f6..59be3d87f86d 100644
--- a/arch/ia64/include/uapi/asm/socket.h
+++ b/arch/ia64/include/uapi/asm/socket.h
@@ -91,4 +91,7 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/uapi/asm/socket.h b/arch/m32r/include/uapi/asm/socket.h
index e8589819c274..7bc4cb273856 100644
--- a/arch/m32r/include/uapi/asm/socket.h
+++ b/arch/m32r/include/uapi/asm/socket.h
@@ -82,4 +82,7 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 2e9ee8c55a10..dec3c850f36b 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -100,4 +100,7 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/mn10300/include/uapi/asm/socket.h b/arch/mn10300/include/uapi/asm/socket.h
index f3492e8c9f70..cab7d6d50051 100644
--- a/arch/mn10300/include/uapi/asm/socket.h
+++ b/arch/mn10300/include/uapi/asm/socket.h
@@ -82,4 +82,7 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index 7984a1cab3da..a5cd40cd8ee1 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -81,4 +81,7 @@
 
 #define SO_INCOMING_CPU		0x402A
 
+#define SO_ATTACH_BPF		0x402B
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/powerpc/include/uapi/asm/socket.h b/arch/powerpc/include/uapi/asm/socket.h
index 3474e4ef166d..c046666038f8 100644
--- a/arch/powerpc/include/uapi/asm/socket.h
+++ b/arch/powerpc/include/uapi/asm/socket.h
@@ -89,4 +89,7 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif	/* _ASM_POWERPC_SOCKET_H */
diff --git a/arch/s390/include/uapi/asm/socket.h b/arch/s390/include/uapi/asm/socket.h
index 8457636c33e1..296942d56e6a 100644
--- a/arch/s390/include/uapi/asm/socket.h
+++ b/arch/s390/include/uapi/asm/socket.h
@@ -88,4 +88,7 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 4a8003a94163..e6a16c40be5f 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -78,6 +78,9 @@
 
 #define SO_INCOMING_CPU		0x0033
 
+#define SO_ATTACH_BPF		0x0034
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 /* Security levels - as per NRL IPv6 - don't actually do anything */
 #define SO_SECURITY_AUTHENTICATION		0x5001
 #define SO_SECURITY_ENCRYPTION_TRANSPORT	0x5002
diff --git a/arch/xtensa/include/uapi/asm/socket.h b/arch/xtensa/include/uapi/asm/socket.h
index c46f6a696849..4120af086160 100644
--- a/arch/xtensa/include/uapi/asm/socket.h
+++ b/arch/xtensa/include/uapi/asm/socket.h
@@ -93,4 +93,7 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif	/* _XTENSA_SOCKET_H */
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 75e94eaa228b..bbfceb756452 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -128,7 +128,11 @@ struct bpf_prog_aux {
 	struct work_struct work;
 };
 
+#ifdef CONFIG_BPF_SYSCALL
 void bpf_prog_put(struct bpf_prog *prog);
+#else
+static inline void bpf_prog_put(struct bpf_prog *prog) {}
+#endif
 struct bpf_prog *bpf_prog_get(u32 ufd);
 /* verify correctness of eBPF program */
 int bpf_check(struct bpf_prog *fp, union bpf_attr *attr);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index ca95abd2bed1..caac2087a4d5 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -381,6 +381,7 @@ int bpf_prog_create(struct bpf_prog **pfp, struct sock_fprog_kern *fprog);
 void bpf_prog_destroy(struct bpf_prog *fp);
 
 int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk);
+int sk_attach_bpf(u32 ufd, struct sock *sk);
 int sk_detach_filter(struct sock *sk);
 
 int bpf_check_classic(const struct sock_filter *filter, unsigned int flen);
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index f541ccefd4ac..5c15c2a5c123 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -84,4 +84,7 @@
 
 #define SO_INCOMING_CPU		49
 
+#define SO_ATTACH_BPF		50
+#define SO_DETACH_BPF		SO_DETACH_FILTER
+
 #endif /* __ASM_GENERIC_SOCKET_H */
diff --git a/net/core/filter.c b/net/core/filter.c
index 647b12265e18..8cc3c03078b3 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -44,6 +44,7 @@
 #include <linux/ratelimit.h>
 #include <linux/seccomp.h>
 #include <linux/if_vlan.h>
+#include <linux/bpf.h>
 
 /**
  *	sk_filter - run a packet through a socket filter
@@ -813,8 +814,12 @@ static void bpf_release_orig_filter(struct bpf_prog *fp)
 
 static void __bpf_prog_release(struct bpf_prog *prog)
 {
-	bpf_release_orig_filter(prog);
-	bpf_prog_free(prog);
+	if (prog->aux->prog_type == BPF_PROG_TYPE_SOCKET_FILTER) {
+		bpf_prog_put(prog);
+	} else {
+		bpf_release_orig_filter(prog);
+		bpf_prog_free(prog);
+	}
 }
 
 static void __sk_filter_release(struct sk_filter *fp)
@@ -1088,6 +1093,94 @@ int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
 }
 EXPORT_SYMBOL_GPL(sk_attach_filter);
 
+#ifdef CONFIG_BPF_SYSCALL
+int sk_attach_bpf(u32 ufd, struct sock *sk)
+{
+	struct sk_filter *fp, *old_fp;
+	struct bpf_prog *prog;
+
+	if (sock_flag(sk, SOCK_FILTER_LOCKED))
+		return -EPERM;
+
+	prog = bpf_prog_get(ufd);
+	if (!prog)
+		return -EINVAL;
+
+	if (prog->aux->prog_type != BPF_PROG_TYPE_SOCKET_FILTER) {
+		/* valid fd, but invalid program type */
+		bpf_prog_put(prog);
+		return -EINVAL;
+	}
+
+	fp = kmalloc(sizeof(*fp), GFP_KERNEL);
+	if (!fp) {
+		bpf_prog_put(prog);
+		return -ENOMEM;
+	}
+	fp->prog = prog;
+
+	atomic_set(&fp->refcnt, 0);
+
+	if (!sk_filter_charge(sk, fp)) {
+		__sk_filter_release(fp);
+		return -ENOMEM;
+	}
+
+	old_fp = rcu_dereference_protected(sk->sk_filter,
+					   sock_owned_by_user(sk));
+	rcu_assign_pointer(sk->sk_filter, fp);
+
+	if (old_fp)
+		sk_filter_uncharge(sk, old_fp);
+
+	return 0;
+}
+
+/* allow socket filters to call
+ * bpf_map_lookup_elem(), bpf_map_update_elem(), bpf_map_delete_elem()
+ */
+static const struct bpf_func_proto *sock_filter_func_proto(enum bpf_func_id func_id)
+{
+	switch (func_id) {
+	case BPF_FUNC_map_lookup_elem:
+		return &bpf_map_lookup_elem_proto;
+	case BPF_FUNC_map_update_elem:
+		return &bpf_map_update_elem_proto;
+	case BPF_FUNC_map_delete_elem:
+		return &bpf_map_delete_elem_proto;
+	default:
+		return NULL;
+	}
+}
+
+static bool sock_filter_is_valid_access(int off, int size, enum bpf_access_type type)
+{
+	/* skb fields cannot be accessed yet */
+	return false;
+}
+
+static struct bpf_verifier_ops sock_filter_ops = {
+	.get_func_proto = sock_filter_func_proto,
+	.is_valid_access = sock_filter_is_valid_access,
+};
+
+static struct bpf_prog_type_list tl = {
+	.ops = &sock_filter_ops,
+	.type = BPF_PROG_TYPE_SOCKET_FILTER,
+};
+
+static int __init register_sock_filter_ops(void)
+{
+	bpf_register_prog_type(&tl);
+	return 0;
+}
+late_initcall(register_sock_filter_ops);
+#else
+int sk_attach_bpf(u32 ufd, struct sock *sk)
+{
+	return -EOPNOTSUPP;
+}
+#endif
 int sk_detach_filter(struct sock *sk)
 {
 	int ret = -ENOENT;
diff --git a/net/core/sock.c b/net/core/sock.c
index 0725cf0cb685..9a56b2000c3f 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -888,6 +888,19 @@ set_rcvbuf:
 		}
 		break;
 
+	case SO_ATTACH_BPF:
+		ret = -EINVAL;
+		if (optlen == sizeof(u32)) {
+			u32 ufd;
+
+			ret = -EFAULT;
+			if (copy_from_user(&ufd, optval, sizeof(ufd)))
+				break;
+
+			ret = sk_attach_bpf(ufd, sk);
+		}
+		break;
+
 	case SO_DETACH_FILTER:
 		ret = sk_detach_filter(sk);
 		break;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 3/6] samples: bpf: example of stateful socket filtering
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1417475199-15950-1-git-send-email-ast@plumgrid.com>

this socket filter example does:
- creates arraymap in kernel with key 4 bytes and value 8 bytes

- loads eBPF program which assumes that packet is IPv4 and loads one byte of
  IP->proto from the packet and uses it as a key in a map

  r0 = skb->data[ETH_HLEN + offsetof(struct iphdr, protocol)];
  *(u32*)(fp - 4) = r0;
  value = bpf_map_lookup_elem(map_fd, fp - 4);
  if (value)
       (*(u64*)value) += 1;

- attaches this program to raw socket

- every second user space reads map[IPPROTO_TCP], map[IPPROTO_UDP], map[IPPROTO_ICMP]
  to see how many packets of given protocol were seen on loopback interface

Usage:
$sudo samples/bpf/sock_example
TCP 0 UDP 0 ICMP 0 packets
TCP 187600 UDP 0 ICMP 4 packets
TCP 376504 UDP 0 ICMP 8 packets
TCP 563116 UDP 0 ICMP 12 packets
TCP 753144 UDP 0 ICMP 16 packets

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
V1->V2:

fixed comments and replaced constants in:
BPF_LD_ABS(BPF_B, 14 + 9 /* R0 = ip->proto */),
with:
BPF_LD_ABS(BPF_B, ETH_HLEN + offsetof(struct iphdr, protocol) /* R0 = ip->proto */),

 samples/bpf/Makefile       |    2 +
 samples/bpf/libbpf.c       |   28 ++++++++++++
 samples/bpf/libbpf.h       |   13 ++++++
 samples/bpf/sock_example.c |  101 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 144 insertions(+)
 create mode 100644 samples/bpf/sock_example.c

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 0718d9ce4619..f46d3492d032 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -3,9 +3,11 @@ obj- := dummy.o
 
 # List of programs to build
 hostprogs-y := test_verifier test_maps
+hostprogs-y += sock_example
 
 test_verifier-objs := test_verifier.o libbpf.o
 test_maps-objs := test_maps.o libbpf.o
+sock_example-objs := sock_example.o libbpf.o
 
 # Tell kbuild to always build the programs
 always := $(hostprogs-y)
diff --git a/samples/bpf/libbpf.c b/samples/bpf/libbpf.c
index 17bb520eb57f..46d50b7ddf79 100644
--- a/samples/bpf/libbpf.c
+++ b/samples/bpf/libbpf.c
@@ -7,6 +7,10 @@
 #include <linux/netlink.h>
 #include <linux/bpf.h>
 #include <errno.h>
+#include <net/ethernet.h>
+#include <net/if.h>
+#include <linux/if_packet.h>
+#include <arpa/inet.h>
 #include "libbpf.h"
 
 static __u64 ptr_to_u64(void *ptr)
@@ -93,3 +97,27 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
 
 	return syscall(__NR_bpf, BPF_PROG_LOAD, &attr, sizeof(attr));
 }
+
+int open_raw_sock(const char *name)
+{
+	struct sockaddr_ll sll;
+	int sock;
+
+	sock = socket(PF_PACKET, SOCK_RAW | SOCK_NONBLOCK | SOCK_CLOEXEC, htons(ETH_P_ALL));
+	if (sock < 0) {
+		printf("cannot create raw socket\n");
+		return -1;
+	}
+
+	memset(&sll, 0, sizeof(sll));
+	sll.sll_family = AF_PACKET;
+	sll.sll_ifindex = if_nametoindex(name);
+	sll.sll_protocol = htons(ETH_P_ALL);
+	if (bind(sock, (struct sockaddr *)&sll, sizeof(sll)) < 0) {
+		printf("bind to %s: %s\n", name, strerror(errno));
+		close(sock);
+		return -1;
+	}
+
+	return sock;
+}
diff --git a/samples/bpf/libbpf.h b/samples/bpf/libbpf.h
index f8678e5f48bf..cc62ad4d95de 100644
--- a/samples/bpf/libbpf.h
+++ b/samples/bpf/libbpf.h
@@ -99,6 +99,16 @@ extern char bpf_log_buf[LOG_BUF_SIZE];
 	BPF_LD_IMM64_RAW(DST, BPF_PSEUDO_MAP_FD, MAP_FD)
 
 
+/* Direct packet access, R0 = *(uint *) (skb->data + imm32) */
+
+#define BPF_LD_ABS(SIZE, IMM)					\
+	((struct bpf_insn) {					\
+		.code  = BPF_LD | BPF_SIZE(SIZE) | BPF_ABS,	\
+		.dst_reg = 0,					\
+		.src_reg = 0,					\
+		.off   = 0,					\
+		.imm   = IMM })
+
 /* Memory load, dst_reg = *(uint *) (src_reg + off16) */
 
 #define BPF_LDX_MEM(SIZE, DST, SRC, OFF)			\
@@ -169,4 +179,7 @@ extern char bpf_log_buf[LOG_BUF_SIZE];
 		.off   = 0,					\
 		.imm   = 0 })
 
+/* create RAW socket and bind to interface 'name' */
+int open_raw_sock(const char *name);
+
 #endif
diff --git a/samples/bpf/sock_example.c b/samples/bpf/sock_example.c
new file mode 100644
index 000000000000..c8ad0404416f
--- /dev/null
+++ b/samples/bpf/sock_example.c
@@ -0,0 +1,101 @@
+/* eBPF example program:
+ * - creates arraymap in kernel with key 4 bytes and value 8 bytes
+ *
+ * - loads eBPF program:
+ *   r0 = skb->data[ETH_HLEN + offsetof(struct iphdr, protocol)];
+ *   *(u32*)(fp - 4) = r0;
+ *   // assuming packet is IPv4, lookup ip->proto in a map
+ *   value = bpf_map_lookup_elem(map_fd, fp - 4);
+ *   if (value)
+ *        (*(u64*)value) += 1;
+ *
+ * - attaches this program to eth0 raw socket
+ *
+ * - every second user space reads map[tcp], map[udp], map[icmp] to see
+ *   how many packets of given protocol were seen on eth0
+ */
+#include <stdio.h>
+#include <unistd.h>
+#include <assert.h>
+#include <linux/bpf.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <sys/socket.h>
+#include <arpa/inet.h>
+#include <linux/if_ether.h>
+#include <linux/ip.h>
+#include <stddef.h>
+#include "libbpf.h"
+
+static int test_sock(void)
+{
+	int sock = -1, map_fd, prog_fd, i, key;
+	long long value = 0, tcp_cnt, udp_cnt, icmp_cnt;
+
+	map_fd = bpf_create_map(BPF_MAP_TYPE_ARRAY, sizeof(key), sizeof(value),
+				256);
+	if (map_fd < 0) {
+		printf("failed to create map '%s'\n", strerror(errno));
+		goto cleanup;
+	}
+
+	struct bpf_insn prog[] = {
+		BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+		BPF_LD_ABS(BPF_B, ETH_HLEN + offsetof(struct iphdr, protocol) /* R0 = ip->proto */),
+		BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4), /* *(u32 *)(fp - 4) = r0 */
+		BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4), /* r2 = fp - 4 */
+		BPF_LD_MAP_FD(BPF_REG_1, map_fd),
+		BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+		BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+		BPF_MOV64_IMM(BPF_REG_1, 1), /* r1 = 1 */
+		BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_DW, BPF_REG_0, BPF_REG_1, 0, 0), /* xadd r0 += r1 */
+		BPF_MOV64_IMM(BPF_REG_0, 0), /* r0 = 0 */
+		BPF_EXIT_INSN(),
+	};
+
+	prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, prog, sizeof(prog),
+				"GPL");
+	if (prog_fd < 0) {
+		printf("failed to load prog '%s'\n", strerror(errno));
+		goto cleanup;
+	}
+
+	sock = open_raw_sock("lo");
+
+	if (setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd,
+		       sizeof(prog_fd)) < 0) {
+		printf("setsockopt %s\n", strerror(errno));
+		goto cleanup;
+	}
+
+	for (i = 0; i < 10; i++) {
+		key = IPPROTO_TCP;
+		assert(bpf_lookup_elem(map_fd, &key, &tcp_cnt) == 0);
+
+		key = IPPROTO_UDP;
+		assert(bpf_lookup_elem(map_fd, &key, &udp_cnt) == 0);
+
+		key = IPPROTO_ICMP;
+		assert(bpf_lookup_elem(map_fd, &key, &icmp_cnt) == 0);
+
+		printf("TCP %lld UDP %lld ICMP %lld packets\n",
+		       tcp_cnt, udp_cnt, icmp_cnt);
+		sleep(1);
+	}
+
+cleanup:
+	/* maps, programs, raw sockets will auto cleanup on process exit */
+	return 0;
+}
+
+int main(void)
+{
+	FILE *f;
+
+	f = popen("ping -c5 localhost", "r");
+	(void)f;
+
+	return test_sock();
+}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 4/6] samples: bpf: elf_bpf file loader
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1417475199-15950-1-git-send-email-ast@plumgrid.com>

simple .o parser and loader using BPF syscall.
.o is a standard ELF generated by LLVM backend

It parses elf file compiled by llvm .c->.o
- parses 'maps' section and creates maps via BPF syscall
- parses 'license' section and passes it to syscall
- parses elf relocations for BPF maps and adjusts BPF_LD_IMM64 insns
  by storing map_fd into insn->imm and marking such insns as BPF_PSEUDO_MAP_FD
- loads eBPF programs via BPF syscall

One ELF file can contain multiple BPF programs.

int load_bpf_file(char *path);
populates prog_fd[] and map_fd[] with FDs received from bpf syscall

bpf_helpers.h - helper functions available to eBPF programs written in C

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
v2: no changes

These helpers and loader are done as separate patch to make eBPF C examples
(that follow in the next patches) to focus on demonstrating programming
of eBPF in restricted C.

 samples/bpf/bpf_helpers.h |   40 +++++++++
 samples/bpf/bpf_load.c    |  203 +++++++++++++++++++++++++++++++++++++++++++++
 samples/bpf/bpf_load.h    |   24 ++++++
 3 files changed, 267 insertions(+)
 create mode 100644 samples/bpf/bpf_helpers.h
 create mode 100644 samples/bpf/bpf_load.c
 create mode 100644 samples/bpf/bpf_load.h

diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
new file mode 100644
index 000000000000..ca0333146006
--- /dev/null
+++ b/samples/bpf/bpf_helpers.h
@@ -0,0 +1,40 @@
+#ifndef __BPF_HELPERS_H
+#define __BPF_HELPERS_H
+
+/* helper macro to place programs, maps, license in
+ * different sections in elf_bpf file. Section names
+ * are interpreted by elf_bpf loader
+ */
+#define SEC(NAME) __attribute__((section(NAME), used))
+
+/* helper functions called from eBPF programs written in C */
+static void *(*bpf_map_lookup_elem)(void *map, void *key) =
+	(void *) BPF_FUNC_map_lookup_elem;
+static int (*bpf_map_update_elem)(void *map, void *key, void *value,
+				  unsigned long long flags) =
+	(void *) BPF_FUNC_map_update_elem;
+static int (*bpf_map_delete_elem)(void *map, void *key) =
+	(void *) BPF_FUNC_map_delete_elem;
+
+/* llvm builtin functions that eBPF C program may use to
+ * emit BPF_LD_ABS and BPF_LD_IND instructions
+ */
+struct sk_buff;
+unsigned long long load_byte(void *skb,
+			     unsigned long long off) asm("llvm.bpf.load.byte");
+unsigned long long load_half(void *skb,
+			     unsigned long long off) asm("llvm.bpf.load.half");
+unsigned long long load_word(void *skb,
+			     unsigned long long off) asm("llvm.bpf.load.word");
+
+/* a helper structure used by eBPF C program
+ * to describe map attributes to elf_bpf loader
+ */
+struct bpf_map_def {
+	unsigned int type;
+	unsigned int key_size;
+	unsigned int value_size;
+	unsigned int max_entries;
+};
+
+#endif
diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
new file mode 100644
index 000000000000..1831d236382b
--- /dev/null
+++ b/samples/bpf/bpf_load.c
@@ -0,0 +1,203 @@
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <libelf.h>
+#include <gelf.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <stdbool.h>
+#include <linux/bpf.h>
+#include <linux/filter.h>
+#include "libbpf.h"
+#include "bpf_helpers.h"
+#include "bpf_load.h"
+
+static char license[128];
+static bool processed_sec[128];
+int map_fd[MAX_MAPS];
+int prog_fd[MAX_PROGS];
+int prog_cnt;
+
+static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
+{
+	int fd;
+	bool is_socket = strncmp(event, "socket", 6) == 0;
+
+	if (!is_socket)
+		/* tracing events tbd */
+		return -1;
+
+	fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER,
+			   prog, size, license);
+
+	if (fd < 0) {
+		printf("bpf_prog_load() err=%d\n%s", errno, bpf_log_buf);
+		return -1;
+	}
+
+	prog_fd[prog_cnt++] = fd;
+
+	return 0;
+}
+
+static int load_maps(struct bpf_map_def *maps, int len)
+{
+	int i;
+
+	for (i = 0; i < len / sizeof(struct bpf_map_def); i++) {
+
+		map_fd[i] = bpf_create_map(maps[i].type,
+					   maps[i].key_size,
+					   maps[i].value_size,
+					   maps[i].max_entries);
+		if (map_fd[i] < 0)
+			return 1;
+	}
+	return 0;
+}
+
+static int get_sec(Elf *elf, int i, GElf_Ehdr *ehdr, char **shname,
+		   GElf_Shdr *shdr, Elf_Data **data)
+{
+	Elf_Scn *scn;
+
+	scn = elf_getscn(elf, i);
+	if (!scn)
+		return 1;
+
+	if (gelf_getshdr(scn, shdr) != shdr)
+		return 2;
+
+	*shname = elf_strptr(elf, ehdr->e_shstrndx, shdr->sh_name);
+	if (!*shname || !shdr->sh_size)
+		return 3;
+
+	*data = elf_getdata(scn, 0);
+	if (!*data || elf_getdata(scn, *data) != NULL)
+		return 4;
+
+	return 0;
+}
+
+static int parse_relo_and_apply(Elf_Data *data, Elf_Data *symbols,
+				GElf_Shdr *shdr, struct bpf_insn *insn)
+{
+	int i, nrels;
+
+	nrels = shdr->sh_size / shdr->sh_entsize;
+
+	for (i = 0; i < nrels; i++) {
+		GElf_Sym sym;
+		GElf_Rel rel;
+		unsigned int insn_idx;
+
+		gelf_getrel(data, i, &rel);
+
+		insn_idx = rel.r_offset / sizeof(struct bpf_insn);
+
+		gelf_getsym(symbols, GELF_R_SYM(rel.r_info), &sym);
+
+		if (insn[insn_idx].code != (BPF_LD | BPF_IMM | BPF_DW)) {
+			printf("invalid relo for insn[%d].code 0x%x\n",
+			       insn_idx, insn[insn_idx].code);
+			return 1;
+		}
+		insn[insn_idx].src_reg = BPF_PSEUDO_MAP_FD;
+		insn[insn_idx].imm = map_fd[sym.st_value / sizeof(struct bpf_map_def)];
+	}
+
+	return 0;
+}
+
+int load_bpf_file(char *path)
+{
+	int fd, i;
+	Elf *elf;
+	GElf_Ehdr ehdr;
+	GElf_Shdr shdr, shdr_prog;
+	Elf_Data *data, *data_prog, *symbols = NULL;
+	char *shname, *shname_prog;
+
+	if (elf_version(EV_CURRENT) == EV_NONE)
+		return 1;
+
+	fd = open(path, O_RDONLY, 0);
+	if (fd < 0)
+		return 1;
+
+	elf = elf_begin(fd, ELF_C_READ, NULL);
+
+	if (!elf)
+		return 1;
+
+	if (gelf_getehdr(elf, &ehdr) != &ehdr)
+		return 1;
+
+	/* scan over all elf sections to get license and map info */
+	for (i = 1; i < ehdr.e_shnum; i++) {
+
+		if (get_sec(elf, i, &ehdr, &shname, &shdr, &data))
+			continue;
+
+		if (0) /* helpful for llvm debugging */
+			printf("section %d:%s data %p size %zd link %d flags %d\n",
+			       i, shname, data->d_buf, data->d_size,
+			       shdr.sh_link, (int) shdr.sh_flags);
+
+		if (strcmp(shname, "license") == 0) {
+			processed_sec[i] = true;
+			memcpy(license, data->d_buf, data->d_size);
+		} else if (strcmp(shname, "maps") == 0) {
+			processed_sec[i] = true;
+			if (load_maps(data->d_buf, data->d_size))
+				return 1;
+		} else if (shdr.sh_type == SHT_SYMTAB) {
+			symbols = data;
+		}
+	}
+
+	/* load programs that need map fixup (relocations) */
+	for (i = 1; i < ehdr.e_shnum; i++) {
+
+		if (get_sec(elf, i, &ehdr, &shname, &shdr, &data))
+			continue;
+		if (shdr.sh_type == SHT_REL) {
+			struct bpf_insn *insns;
+
+			if (get_sec(elf, shdr.sh_info, &ehdr, &shname_prog,
+				    &shdr_prog, &data_prog))
+				continue;
+
+			insns = (struct bpf_insn *) data_prog->d_buf;
+
+			processed_sec[shdr.sh_info] = true;
+			processed_sec[i] = true;
+
+			if (parse_relo_and_apply(data, symbols, &shdr, insns))
+				continue;
+
+			if (memcmp(shname_prog, "events/", 7) == 0 ||
+			    memcmp(shname_prog, "socket", 6) == 0)
+				load_and_attach(shname_prog, insns, data_prog->d_size);
+		}
+	}
+
+	/* load programs that don't use maps */
+	for (i = 1; i < ehdr.e_shnum; i++) {
+
+		if (processed_sec[i])
+			continue;
+
+		if (get_sec(elf, i, &ehdr, &shname, &shdr, &data))
+			continue;
+
+		if (memcmp(shname, "events/", 7) == 0 ||
+		    memcmp(shname, "socket", 6) == 0)
+			load_and_attach(shname, data->d_buf, data->d_size);
+	}
+
+	close(fd);
+	return 0;
+}
diff --git a/samples/bpf/bpf_load.h b/samples/bpf/bpf_load.h
new file mode 100644
index 000000000000..27789a34f5e6
--- /dev/null
+++ b/samples/bpf/bpf_load.h
@@ -0,0 +1,24 @@
+#ifndef __BPF_LOAD_H
+#define __BPF_LOAD_H
+
+#define MAX_MAPS 32
+#define MAX_PROGS 32
+
+extern int map_fd[MAX_MAPS];
+extern int prog_fd[MAX_PROGS];
+
+/* parses elf file compiled by llvm .c->.o
+ * . parses 'maps' section and creates maps via BPF syscall
+ * . parses 'license' section and passes it to syscall
+ * . parses elf relocations for BPF maps and adjusts BPF_LD_IMM64 insns by
+ *   storing map_fd into insn->imm and marking such insns as BPF_PSEUDO_MAP_FD
+ * . loads eBPF programs via BPF syscall
+ *
+ * One ELF file can contain multiple BPF programs which will be loaded
+ * and their FDs stored stored in prog_fd array
+ *
+ * returns zero on success
+ */
+int load_bpf_file(char *path);
+
+#endif
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 5/6] samples: bpf: trivial eBPF program in C
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1417475199-15950-1-git-send-email-ast@plumgrid.com>

this example does the same task as previous socket example
in assembler, but this one does it in C.

eBPF program in kernel does:
    /* assume that packet is IPv4, load one byte of IP->proto */
    int index = load_byte(skb, ETH_HLEN + offsetof(struct iphdr, protocol));
    long *value;

    value = bpf_map_lookup_elem(&my_map, &index);
    if (value)
        __sync_fetch_and_add(value, 1);

Corresponding user space reads map[tcp], map[udp], map[icmp]
and prints protocol stats every second

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
V1->V2:

replaced constants in:
load_byte(skb, 14 + 9)
with:
load_byte(skb, ETH_HLEN + offsetof(struct iphdr, protocol))

 samples/bpf/Makefile       |   14 +++++++++++++
 samples/bpf/libbpf.h       |    2 +-
 samples/bpf/sockex1_kern.c |   25 ++++++++++++++++++++++
 samples/bpf/sockex1_user.c |   49 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 samples/bpf/sockex1_kern.c
 create mode 100644 samples/bpf/sockex1_user.c

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index f46d3492d032..770d145186c3 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -4,12 +4,26 @@ obj- := dummy.o
 # List of programs to build
 hostprogs-y := test_verifier test_maps
 hostprogs-y += sock_example
+hostprogs-y += sockex1
 
 test_verifier-objs := test_verifier.o libbpf.o
 test_maps-objs := test_maps.o libbpf.o
 sock_example-objs := sock_example.o libbpf.o
+sockex1-objs := bpf_load.o libbpf.o sockex1_user.o
 
 # Tell kbuild to always build the programs
 always := $(hostprogs-y)
+always += sockex1_kern.o
 
 HOSTCFLAGS += -I$(objtree)/usr/include
+
+HOSTCFLAGS_bpf_load.o += -I$(objtree)/usr/include -Wno-unused-variable
+HOSTLOADLIBES_sockex1 += -lelf
+
+# point this to your LLVM backend with bpf support
+LLC=$(srctree)/tools/bpf/llvm/bld/Debug+Asserts/bin/llc
+
+%.o: %.c
+	clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
+		-D__KERNEL__ -Wno-unused-value -Wno-pointer-sign \
+		-O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
diff --git a/samples/bpf/libbpf.h b/samples/bpf/libbpf.h
index cc62ad4d95de..58c5fe1bdba1 100644
--- a/samples/bpf/libbpf.h
+++ b/samples/bpf/libbpf.h
@@ -15,7 +15,7 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
 		  const struct bpf_insn *insns, int insn_len,
 		  const char *license);
 
-#define LOG_BUF_SIZE 8192
+#define LOG_BUF_SIZE 65536
 extern char bpf_log_buf[LOG_BUF_SIZE];
 
 /* ALU ops on registers, bpf_add|sub|...: dst_reg += src_reg */
diff --git a/samples/bpf/sockex1_kern.c b/samples/bpf/sockex1_kern.c
new file mode 100644
index 000000000000..066892662915
--- /dev/null
+++ b/samples/bpf/sockex1_kern.c
@@ -0,0 +1,25 @@
+#include <uapi/linux/bpf.h>
+#include <uapi/linux/if_ether.h>
+#include <uapi/linux/ip.h>
+#include "bpf_helpers.h"
+
+struct bpf_map_def SEC("maps") my_map = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(u32),
+	.value_size = sizeof(long),
+	.max_entries = 256,
+};
+
+SEC("socket1")
+int bpf_prog1(struct sk_buff *skb)
+{
+	int index = load_byte(skb, ETH_HLEN + offsetof(struct iphdr, protocol));
+	long *value;
+
+	value = bpf_map_lookup_elem(&my_map, &index);
+	if (value)
+		__sync_fetch_and_add(value, 1);
+
+	return 0;
+}
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/sockex1_user.c b/samples/bpf/sockex1_user.c
new file mode 100644
index 000000000000..34a443ff3831
--- /dev/null
+++ b/samples/bpf/sockex1_user.c
@@ -0,0 +1,49 @@
+#include <stdio.h>
+#include <assert.h>
+#include <linux/bpf.h>
+#include "libbpf.h"
+#include "bpf_load.h"
+#include <unistd.h>
+#include <arpa/inet.h>
+
+int main(int ac, char **argv)
+{
+	char filename[256];
+	FILE *f;
+	int i, sock;
+
+	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+
+	if (load_bpf_file(filename)) {
+		printf("%s", bpf_log_buf);
+		return 1;
+	}
+
+	sock = open_raw_sock("lo");
+
+	assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
+			  sizeof(prog_fd[0])) == 0);
+
+	f = popen("ping -c5 localhost", "r");
+	(void) f;
+
+	for (i = 0; i < 5; i++) {
+		long long tcp_cnt, udp_cnt, icmp_cnt;
+		int key;
+
+		key = IPPROTO_TCP;
+		assert(bpf_lookup_elem(map_fd[0], &key, &tcp_cnt) == 0);
+
+		key = IPPROTO_UDP;
+		assert(bpf_lookup_elem(map_fd[0], &key, &udp_cnt) == 0);
+
+		key = IPPROTO_ICMP;
+		assert(bpf_lookup_elem(map_fd[0], &key, &icmp_cnt) == 0);
+
+		printf("TCP %lld UDP %lld ICMP %lld packets\n",
+		       tcp_cnt, udp_cnt, icmp_cnt);
+		sleep(1);
+	}
+
+	return 0;
+}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v2 net-next 6/6] samples: bpf: large eBPF program in C
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
  To: David S. Miller
  Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
	linux-kernel
In-Reply-To: <1417475199-15950-1-git-send-email-ast@plumgrid.com>

sockex2_kern.c is purposefully large eBPF program in C.
llvm compiles ~200 lines of C code into ~300 eBPF instructions.

It's similar to __skb_flow_dissect() to demonstrate that complex packet parsing
can be done by eBPF.
Then it uses (struct flow_keys)->dst IP address (or hash of ipv6 dst) to keep
stats of number of packets per IP.
User space loads eBPF program, attaches it to loopback interface and prints
dest_ip->#packets stats every second.

Usage:
$sudo samples/bpf/sockex2
ip 127.0.0.1 count 19
ip 127.0.0.1 count 178115
ip 127.0.0.1 count 369437
ip 127.0.0.1 count 559841
ip 127.0.0.1 count 750539

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
v2: no changes

 samples/bpf/Makefile       |    4 +
 samples/bpf/sockex2_kern.c |  215 ++++++++++++++++++++++++++++++++++++++++++++
 samples/bpf/sockex2_user.c |   44 +++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 samples/bpf/sockex2_kern.c
 create mode 100644 samples/bpf/sockex2_user.c

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 770d145186c3..b5b3600dcdf5 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -5,20 +5,24 @@ obj- := dummy.o
 hostprogs-y := test_verifier test_maps
 hostprogs-y += sock_example
 hostprogs-y += sockex1
+hostprogs-y += sockex2
 
 test_verifier-objs := test_verifier.o libbpf.o
 test_maps-objs := test_maps.o libbpf.o
 sock_example-objs := sock_example.o libbpf.o
 sockex1-objs := bpf_load.o libbpf.o sockex1_user.o
+sockex2-objs := bpf_load.o libbpf.o sockex2_user.o
 
 # Tell kbuild to always build the programs
 always := $(hostprogs-y)
 always += sockex1_kern.o
+always += sockex2_kern.o
 
 HOSTCFLAGS += -I$(objtree)/usr/include
 
 HOSTCFLAGS_bpf_load.o += -I$(objtree)/usr/include -Wno-unused-variable
 HOSTLOADLIBES_sockex1 += -lelf
+HOSTLOADLIBES_sockex2 += -lelf
 
 # point this to your LLVM backend with bpf support
 LLC=$(srctree)/tools/bpf/llvm/bld/Debug+Asserts/bin/llc
diff --git a/samples/bpf/sockex2_kern.c b/samples/bpf/sockex2_kern.c
new file mode 100644
index 000000000000..6f0135f0f217
--- /dev/null
+++ b/samples/bpf/sockex2_kern.c
@@ -0,0 +1,215 @@
+#include <uapi/linux/bpf.h>
+#include "bpf_helpers.h"
+#include <uapi/linux/in.h>
+#include <uapi/linux/if.h>
+#include <uapi/linux/if_ether.h>
+#include <uapi/linux/ip.h>
+#include <uapi/linux/ipv6.h>
+#include <uapi/linux/if_tunnel.h>
+#define IP_MF		0x2000
+#define IP_OFFSET	0x1FFF
+
+struct vlan_hdr {
+	__be16 h_vlan_TCI;
+	__be16 h_vlan_encapsulated_proto;
+};
+
+struct flow_keys {
+	__be32 src;
+	__be32 dst;
+	union {
+		__be32 ports;
+		__be16 port16[2];
+	};
+	__u16 thoff;
+	__u8 ip_proto;
+};
+
+static inline int proto_ports_offset(__u64 proto)
+{
+	switch (proto) {
+	case IPPROTO_TCP:
+	case IPPROTO_UDP:
+	case IPPROTO_DCCP:
+	case IPPROTO_ESP:
+	case IPPROTO_SCTP:
+	case IPPROTO_UDPLITE:
+		return 0;
+	case IPPROTO_AH:
+		return 4;
+	default:
+		return 0;
+	}
+}
+
+static inline int ip_is_fragment(struct sk_buff *ctx, __u64 nhoff)
+{
+	return load_half(ctx, nhoff + offsetof(struct iphdr, frag_off))
+		& (IP_MF | IP_OFFSET);
+}
+
+static inline __u32 ipv6_addr_hash(struct sk_buff *ctx, __u64 off)
+{
+	__u64 w0 = load_word(ctx, off);
+	__u64 w1 = load_word(ctx, off + 4);
+	__u64 w2 = load_word(ctx, off + 8);
+	__u64 w3 = load_word(ctx, off + 12);
+
+	return (__u32)(w0 ^ w1 ^ w2 ^ w3);
+}
+
+static inline __u64 parse_ip(struct sk_buff *skb, __u64 nhoff, __u64 *ip_proto,
+			     struct flow_keys *flow)
+{
+	__u64 verlen;
+
+	if (unlikely(ip_is_fragment(skb, nhoff)))
+		*ip_proto = 0;
+	else
+		*ip_proto = load_byte(skb, nhoff + offsetof(struct iphdr, protocol));
+
+	if (*ip_proto != IPPROTO_GRE) {
+		flow->src = load_word(skb, nhoff + offsetof(struct iphdr, saddr));
+		flow->dst = load_word(skb, nhoff + offsetof(struct iphdr, daddr));
+	}
+
+	verlen = load_byte(skb, nhoff + 0/*offsetof(struct iphdr, ihl)*/);
+	if (likely(verlen == 0x45))
+		nhoff += 20;
+	else
+		nhoff += (verlen & 0xF) << 2;
+
+	return nhoff;
+}
+
+static inline __u64 parse_ipv6(struct sk_buff *skb, __u64 nhoff, __u64 *ip_proto,
+			       struct flow_keys *flow)
+{
+	*ip_proto = load_byte(skb,
+			      nhoff + offsetof(struct ipv6hdr, nexthdr));
+	flow->src = ipv6_addr_hash(skb,
+				   nhoff + offsetof(struct ipv6hdr, saddr));
+	flow->dst = ipv6_addr_hash(skb,
+				   nhoff + offsetof(struct ipv6hdr, daddr));
+	nhoff += sizeof(struct ipv6hdr);
+
+	return nhoff;
+}
+
+static inline bool flow_dissector(struct sk_buff *skb, struct flow_keys *flow)
+{
+	__u64 nhoff = ETH_HLEN;
+	__u64 ip_proto;
+	__u64 proto = load_half(skb, 12);
+	int poff;
+
+	if (proto == ETH_P_8021AD) {
+		proto = load_half(skb, nhoff + offsetof(struct vlan_hdr,
+							h_vlan_encapsulated_proto));
+		nhoff += sizeof(struct vlan_hdr);
+	}
+
+	if (proto == ETH_P_8021Q) {
+		proto = load_half(skb, nhoff + offsetof(struct vlan_hdr,
+							h_vlan_encapsulated_proto));
+		nhoff += sizeof(struct vlan_hdr);
+	}
+
+	if (likely(proto == ETH_P_IP))
+		nhoff = parse_ip(skb, nhoff, &ip_proto, flow);
+	else if (proto == ETH_P_IPV6)
+		nhoff = parse_ipv6(skb, nhoff, &ip_proto, flow);
+	else
+		return false;
+
+	switch (ip_proto) {
+	case IPPROTO_GRE: {
+		struct gre_hdr {
+			__be16 flags;
+			__be16 proto;
+		};
+
+		__u64 gre_flags = load_half(skb,
+					    nhoff + offsetof(struct gre_hdr, flags));
+		__u64 gre_proto = load_half(skb,
+					    nhoff + offsetof(struct gre_hdr, proto));
+
+		if (gre_flags & (GRE_VERSION|GRE_ROUTING))
+			break;
+
+		proto = gre_proto;
+		nhoff += 4;
+		if (gre_flags & GRE_CSUM)
+			nhoff += 4;
+		if (gre_flags & GRE_KEY)
+			nhoff += 4;
+		if (gre_flags & GRE_SEQ)
+			nhoff += 4;
+
+		if (proto == ETH_P_8021Q) {
+			proto = load_half(skb,
+					  nhoff + offsetof(struct vlan_hdr,
+							   h_vlan_encapsulated_proto));
+			nhoff += sizeof(struct vlan_hdr);
+		}
+
+		if (proto == ETH_P_IP)
+			nhoff = parse_ip(skb, nhoff, &ip_proto, flow);
+		else if (proto == ETH_P_IPV6)
+			nhoff = parse_ipv6(skb, nhoff, &ip_proto, flow);
+		else
+			return false;
+		break;
+	}
+	case IPPROTO_IPIP:
+		nhoff = parse_ip(skb, nhoff, &ip_proto, flow);
+		break;
+	case IPPROTO_IPV6:
+		nhoff = parse_ipv6(skb, nhoff, &ip_proto, flow);
+		break;
+	default:
+		break;
+	}
+
+	flow->ip_proto = ip_proto;
+	poff = proto_ports_offset(ip_proto);
+	if (poff >= 0) {
+		nhoff += poff;
+		flow->ports = load_word(skb, nhoff);
+	}
+
+	flow->thoff = (__u16) nhoff;
+
+	return true;
+}
+
+struct bpf_map_def SEC("maps") hash_map = {
+	.type = BPF_MAP_TYPE_HASH,
+	.key_size = sizeof(__be32),
+	.value_size = sizeof(long),
+	.max_entries = 1024,
+};
+
+SEC("socket2")
+int bpf_prog2(struct sk_buff *skb)
+{
+	struct flow_keys flow;
+	long *value;
+	u32 key;
+
+	if (!flow_dissector(skb, &flow))
+		return 0;
+
+	key = flow.dst;
+	value = bpf_map_lookup_elem(&hash_map, &key);
+	if (value) {
+		__sync_fetch_and_add(value, 1);
+	} else {
+		long val = 1;
+
+		bpf_map_update_elem(&hash_map, &key, &val, BPF_ANY);
+	}
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/sockex2_user.c b/samples/bpf/sockex2_user.c
new file mode 100644
index 000000000000..d2d5f5a790d3
--- /dev/null
+++ b/samples/bpf/sockex2_user.c
@@ -0,0 +1,44 @@
+#include <stdio.h>
+#include <assert.h>
+#include <linux/bpf.h>
+#include "libbpf.h"
+#include "bpf_load.h"
+#include <unistd.h>
+#include <arpa/inet.h>
+
+int main(int ac, char **argv)
+{
+	char filename[256];
+	FILE *f;
+	int i, sock;
+
+	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+
+	if (load_bpf_file(filename)) {
+		printf("%s", bpf_log_buf);
+		return 1;
+	}
+
+	sock = open_raw_sock("lo");
+
+	assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
+			  sizeof(prog_fd[0])) == 0);
+
+	f = popen("ping -c5 localhost", "r");
+	(void) f;
+
+	for (i = 0; i < 5; i++) {
+		int key = 0, next_key;
+		long long value;
+
+		while (bpf_get_next_key(map_fd[0], &key, &next_key) == 0) {
+			bpf_lookup_elem(map_fd[0], &next_key, &value);
+			printf("ip %s count %lld\n",
+			       inet_ntoa((struct in_addr){htonl(next_key)}),
+			       value);
+			key = next_key;
+		}
+		sleep(1);
+	}
+	return 0;
+}
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH v2 net-next] net: bcmgenet: enable driver to work without a device tree
From: Florian Fainelli @ 2014-12-01 23:23 UTC (permalink / raw)
  To: Petri Gynther, netdev; +Cc: davem, Kevin Cernekee
In-Reply-To: <20141201220828.942A7220726@puck.mtv.corp.google.com>

On 01/12/14 14:08, Petri Gynther wrote:
> Broadcom 7xxx MIPS-based STB platforms do not use device trees.
> Modify bcmgenet driver so that it can be used on those platforms.

Well, that statement is technically not true anymore thanks to Kevin's
recent efforts, but this is not exactly what we have been
advertising/supporting so far.

Looks mostly good to me, some minor nits below:

> 
> Signed-off-by: Petri Gynther <pgynther@google.com>
> ---
[snip]
> +	if (dn) {
> +		of_id = of_match_node(bcmgenet_match, dn);
> +		if (!of_id)
> +			return -EINVAL;
> +	} else {
> +		of_id = NULL;
> +	}

You could probably get way with this else condition by assigning of_id
to NULL by default.

[snip]

> +		if (pd->phy_addr >= 0 && pd->phy_addr < PHY_MAX_ADDR) {
> +			phydev = mdio->phy_map[pd->phy_addr];
> +		} else {
> +			phydev = NULL;
> +			for (i = 0; i < PHY_MAX_ADDR; i++) {
> +				if (mdio->phy_map[i]) {
> +					phydev = mdio->phy_map[i];
> +					break;
> +				}
> +			}

phy_find_first() might provide a shorter version of this.

> +		}
> +
> +		if (!phydev) {
> +			dev_err(kdev, "failed to register PHY device\n");
> +			mdiobus_unregister(mdio);
> +			return -ENODEV;
> +		}
> +	} else {
> +		/*
> +		 * MoCA port or no MDIO access.
> +		 * Use 1000/FD fixed PHY to represent the link layer.
> +		 */
> +		struct fixed_phy_status fphy_status = {
> +			.link = 1,
> +			.duplex = DUPLEX_FULL,
> +			.speed = SPEED_1000,
> +			.pause = 0,
> +			.asym_pause = 0,
> +		};
> +
> +		phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
> +		if (!phydev || IS_ERR(phydev)) {
> +			dev_err(kdev, "failed to register fixed PHY device\n");
> +			return -ENODEV;
> +		}

This is typically done by platform code (not necessarily for good
reasons though) but I cannot seen any problems with doing this here as well.

[snip]

> diff --git a/include/linux/platform_data/bcmgenet.h b/include/linux/platform_data/bcmgenet.h
> new file mode 100644
> index 0000000..3660133
> --- /dev/null
> +++ b/include/linux/platform_data/bcmgenet.h
> @@ -0,0 +1,18 @@
> +#ifndef __LINUX_PLATFORM_DATA_BCMGENET_H__
> +#define __LINUX_PLATFORM_DATA_BCMGENET_H__
> +
> +#include <linux/compiler.h>

That include does not look necessary, you might want linux/types.h for
"u8" though.

> +#include <linux/if_ether.h>
> +
> +struct bcmgenet_platform_data {
> +	void __iomem *base_reg;
> +	int           irq0;
> +	int           irq1;

These 3 members are unused and should be communicated to the driver as
resources, not side parameters, can you get rid of them?

> +	bool          mdio_enabled;
> +	int           phy_type;

This is essentially phy_interface_t, so let's use that type directly here.

> +	int           phy_addr;
> +	u8            macaddr[ETH_ALEN];
> +	int           genet_version;

That is also an enum, if that helps, you could move it from
drivers/net/ethernet/broadcom/bcmgenet.h there as well.
--
Florian

^ permalink raw reply

* Re: [PATCH v2 net-next] net: bcmgenet: enable driver to work without a device tree
From: Florian Fainelli @ 2014-12-01 23:26 UTC (permalink / raw)
  To: Petri Gynther, netdev; +Cc: davem, Kevin Cernekee
In-Reply-To: <547CF87A.3010605@gmail.com>

On 01/12/14 15:23, Florian Fainelli wrote:
> On 01/12/14 14:08, Petri Gynther wrote:
>> Broadcom 7xxx MIPS-based STB platforms do not use device trees.
>> Modify bcmgenet driver so that it can be used on those platforms.
> 
> Well, that statement is technically not true anymore thanks to Kevin's
> recent efforts, but this is not exactly what we have been
> advertising/supporting so far.
> 
> Looks mostly good to me, some minor nits below:
> 
>>
>> Signed-off-by: Petri Gynther <pgynther@google.com>
>> ---
> [snip]
>> +	if (dn) {
>> +		of_id = of_match_node(bcmgenet_match, dn);
>> +		if (!of_id)
>> +			return -EINVAL;
>> +	} else {
>> +		of_id = NULL;
>> +	}
> 
> You could probably get way with this else condition by assigning of_id
> to NULL by default.
> 
> [snip]
> 
>> +		if (pd->phy_addr >= 0 && pd->phy_addr < PHY_MAX_ADDR) {
>> +			phydev = mdio->phy_map[pd->phy_addr];
>> +		} else {
>> +			phydev = NULL;
>> +			for (i = 0; i < PHY_MAX_ADDR; i++) {
>> +				if (mdio->phy_map[i]) {
>> +					phydev = mdio->phy_map[i];
>> +					break;
>> +				}
>> +			}
> 
> phy_find_first() might provide a shorter version of this.
> 
>> +		}
>> +
>> +		if (!phydev) {
>> +			dev_err(kdev, "failed to register PHY device\n");
>> +			mdiobus_unregister(mdio);
>> +			return -ENODEV;
>> +		}
>> +	} else {
>> +		/*
>> +		 * MoCA port or no MDIO access.
>> +		 * Use 1000/FD fixed PHY to represent the link layer.
>> +		 */
>> +		struct fixed_phy_status fphy_status = {
>> +			.link = 1,
>> +			.duplex = DUPLEX_FULL,
>> +			.speed = SPEED_1000,
>> +			.pause = 0,
>> +			.asym_pause = 0,
>> +		};
>> +
>> +		phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
>> +		if (!phydev || IS_ERR(phydev)) {
>> +			dev_err(kdev, "failed to register fixed PHY device\n");
>> +			return -ENODEV;
>> +		}
> 
> This is typically done by platform code (not necessarily for good
> reasons though) but I cannot seen any problems with doing this here as well.

Well actually there could be one issue, we don't want to assume
1Gbits/sec here, we would want to be communicated a speed information
(typically ETH0_SPEED) in CFE so we can program the link parameters
accordingly, just in case someone interfaces this GENET instance with
e.g: a non-MDIO managed external 10/100 switch.

> 
> [snip]
> 
>> diff --git a/include/linux/platform_data/bcmgenet.h b/include/linux/platform_data/bcmgenet.h
>> new file mode 100644
>> index 0000000..3660133
>> --- /dev/null
>> +++ b/include/linux/platform_data/bcmgenet.h
>> @@ -0,0 +1,18 @@
>> +#ifndef __LINUX_PLATFORM_DATA_BCMGENET_H__
>> +#define __LINUX_PLATFORM_DATA_BCMGENET_H__
>> +
>> +#include <linux/compiler.h>
> 
> That include does not look necessary, you might want linux/types.h for
> "u8" though.
> 
>> +#include <linux/if_ether.h>
>> +
>> +struct bcmgenet_platform_data {
>> +	void __iomem *base_reg;
>> +	int           irq0;
>> +	int           irq1;
> 
> These 3 members are unused and should be communicated to the driver as
> resources, not side parameters, can you get rid of them?
> 
>> +	bool          mdio_enabled;
>> +	int           phy_type;
> 
> This is essentially phy_interface_t, so let's use that type directly here.
> 
>> +	int           phy_addr;
>> +	u8            macaddr[ETH_ALEN];
>> +	int           genet_version;
> 
> That is also an enum, if that helps, you could move it from
> drivers/net/ethernet/broadcom/bcmgenet.h there as well.
> --
> Florian
> 

^ permalink raw reply

* Re: [PATCHv2 net] i40e: Implement ndo_gso_check()
From: Joe Stringer @ 2014-12-01 23:35 UTC (permalink / raw)
  To: Jesse Gross
  Cc: netdev, Shannon Nelson, Brandeburg, Jesse, Jeff Kirsher,
	Tom Herbert, linux.nics, Linux Kernel Mailing List
In-Reply-To: <CANr6G5zrA6NUtcyhX1cwEO1Hiq9jnarB36P6m5eoHUFqWWc26Q@mail.gmail.com>

On 21 November 2014 at 09:59, Joe Stringer <joestringer@nicira.com> wrote:
> On 20 November 2014 16:19, Jesse Gross <jesse@nicira.com> wrote:
>> I don't know if we need to have the check at all for IPIP though -
>> after all the driver doesn't expose support for it all (actually it
>> doesn't expose GRE either). This raises kind of an interesting
>> question about the checks though - it's pretty easy to add support to
>> the driver for a new GSO type (and I imagine that people will be
>> adding GRE soon) and forget to update the check.
>
> If the check is more conservative, then testing would show that it's
> not working and lead people to figure out why (and update the check).

More concretely, one suggestion would be something like following at
the start of each gso_check():

+       const int supported = SKB_GSO_TCPV4 | SKB_GSO_TCPV6 | SKB_GSO_FCOE |
+                             SKB_GSO_UDP | SKB_GSO_UDP_TUNNEL;
+
+       if (skb_shinfo(skb)->gso_type & ~supported)
+               return false;

..followed by checking the specifics for each. So far the patches have
only been concerned with further checking on UDP tunnels.

^ permalink raw reply

* Re: ovs datapath complains on unexpected mask for netlink commands
From: Jesse Gross @ 2014-12-01 23:39 UTC (permalink / raw)
  To: Or Gerlitz, Pravin Shelar; +Cc: netdev@vger.kernel.org
In-Reply-To: <54775D61.6090300@mellanox.com>

On Thu, Nov 27, 2014 at 9:20 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> Hi Jesse,
>
> I am running with net-next as of commit 2ad7bf36 "ipvlan: Initial check-in
> [...]"
> and user-space OVS v2.0.90 built from the upstream git.
>
> I suddenly realized that my kernel logs gets filled with the below
> prints which effectively take place for each ovs-vsctl command I run...
>
> openvswitch: netlink: Unexpected mask (mask=1205c, allowed=19805c)
> openvswitch: netlink: Unexpected mask (mask=1205c, allowed=19805c)
> openvswitch: netlink: Unexpected mask (mask=1205c, allowed=19805c)
> openvswitch: netlink: Unexpected mask (mask=1205c, allowed=19805c)
> openvswitch: netlink: Unexpected mask (mask=1205c, allowed=19805c)

Thanks I see the problem. It looks like Pravin just sent out a patch
to fix this issue last night - "[PATCH net] openvswitch: Fix flow mask
validation.".

^ permalink raw reply

* Re: [PATCH v2 1/6] GMAC: add driver for Rockchip RK3288 SoCs integrated GMAC
From: Heiko Stübner @ 2014-12-01 23:44 UTC (permalink / raw)
  To: Roger Chen
  Cc: peppe.cavallaro, netdev, linux-kernel, linux-rockchip, kever.yang,
	eddie.cai
In-Reply-To: <1417056728-3642-1-git-send-email-roger.chen@rock-chips.com>

Hi Roger,

the comments inline are a rough first review. I hope to get a clearer picture 
for the stuff I'm not sure about in v3 once the big issues are fixed.


Am Donnerstag, 27. November 2014, 10:52:08 schrieb Roger Chen:
> This driver is based on stmmac driver.
> 
> modification based on Giuseppe CAVALLARO's suggestion:
> 1. use BIT()
> 
> 	> +/*RK3288_GRF_SOC_CON3*/
> 	> +#define GMAC_TXCLK_DLY_ENABLE   ((0x4000 << 16) | (0x4000))
> 	> +#define GMAC_TXCLK_DLY_DISABLE  ((0x4000 << 16) | (0x0000))
> 
> 	...
> 
> 	why do not use BIT and BIT_MASK where possible?
> 
> 	===>after modification:
> 
> 	#define GRF_BIT(nr)     (BIT(nr) | BIT(nr+16))
> 	#define GRF_CLR_BIT(nr) (BIT(nr+16))
> 	#define GMAC_TXCLK_DLY_ENABLE   GRF_BIT(14)
> 	#define GMAC_TXCLK_DLY_DISABLE  GRF_CLR_BIT(14)
> 	...
> 2.
> 
> 	> +    regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
> 	> +             GMAC_PHY_INTF_SEL_RGMII);
> 	> +    regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
> 	> +             GMAC_RMII_MODE_CLR);
> 
> 	maybe you could perform just one write unless there is some HW
> 	constraint.
> 
> 	===>after modification:
> 
> 	regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
> 			 GMAC_PHY_INTF_SEL_RGMII | GMAC_RMII_MODE_CLR);
> 
> 3. use macros
> 
> 	> +    regmap_write(bsp_priv->grf, RK3288_GRF_GPIO3D_E, 0xFFFFFFFF);
> 	> +    regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4B_E,
> 	> +             0x3<<2<<16 | 0x3<<2);
> 
> 	pls use macros, these shift sequence is really help to decode
> 
> 	===>after modification:
> 
> 	regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4A_E, GPIO4A_12MA);
> 	regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4B_E, GPIO4B_2_12MA);
> 
> 4. remove grf fail check in rk_gmac_setup()
> 
> 	> +    if (IS_ERR(bsp_priv->grf))
> 	> +        dev_err(&pdev->dev, "Missing rockchip,grf property\n");
> 
> 	I wonder if you can fail on here and save all the check in
> 	set_rgmii_speed etc.
> 	Maybe this can be considered a mandatory property for the glue-logic.
> 
> 5. remove .tx_coe=1
> 
> 	> +const struct stmmac_of_data rk_gmac_data = {
> 	> +    .has_gmac = 1,
> 	> +    .tx_coe = 1,
> 
> 	FYI, on new gmac there is the HW capability register to dinamically
> 	provide you if coe is supported.
> 
> 	IMO you should add the OF "compatible" string and in case of mac
> 	newer than the 3.50a you can remove coe.

changelogs like these, should be compact and also not be in the commit message 
itself, but in the "comment"-section below the "---" and before the diffstat.


> 
> Signed-off-by: Roger Chen <roger.chen@rock-chips.com>
> ---

changelog here ... the commonly used pattern is something like

changes since v2:
- ...
- ...

changes since v1:
- ...

>  drivers/net/ethernet/stmicro/stmmac/Makefile       |    2 +-
>  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c     |  636
> ++++++++++++++++++++ .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  
>  3 +
>  .../net/ethernet/stmicro/stmmac/stmmac_platform.h  |    1 +
>  4 files changed, 641 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile
> b/drivers/net/ethernet/stmicro/stmmac/Makefile index ac4d562..73c2715
> 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> @@ -6,7 +6,7 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o
> ring_mode.o	\
> 
>  obj-$(CONFIG_STMMAC_PLATFORM) += stmmac-platform.o
>  stmmac-platform-objs:= stmmac_platform.o dwmac-meson.o dwmac-sunxi.o	\
> -		       dwmac-sti.o dwmac-socfpga.o
> +		       dwmac-sti.o dwmac-socfpga.o dwmac-rk.o
> 
>  obj-$(CONFIG_STMMAC_PCI) += stmmac-pci.o
>  stmmac-pci-objs:= stmmac_pci.o
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c new file mode 100644
> index 0000000..870563f
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -0,0 +1,636 @@
> +/**
> + * dwmac-rk.c - Rockchip RK3288 DWMAC specific glue layer
> + *
> + * Copyright (C) 2014 Chen-Zhi (Roger Chen)
> + *
> + * Chen-Zhi (Roger Chen)  <roger.chen@rock-chips.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/stmmac.h>
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/phy.h>
> +#include <linux/of_net.h>
> +#include <linux/gpio.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/delay.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +
> +struct rk_priv_data {
> +	struct platform_device *pdev;
> +	int phy_iface;
> +	bool power_ctrl_by_pmu;
> +	char pmu_regulator[32];
> +	int pmu_enable_level;
> +
> +	int power_io;
> +	int power_io_level;
> +	int reset_io;
> +	int reset_io_level;
> +	int phyirq_io;
> +	int phyirq_io_level;
> +
> +	bool clk_enabled;
> +	bool clock_input;
> +
> +	struct clk *clk_mac;
> +	struct clk *clk_mac_pll;
> +	struct clk *gmac_clkin;
> +	struct clk *mac_clk_rx;
> +	struct clk *mac_clk_tx;
> +	struct clk *clk_mac_ref;
> +	struct clk *clk_mac_refout;
> +	struct clk *aclk_mac;
> +	struct clk *pclk_mac;
> +
> +	int tx_delay;
> +	int rx_delay;
> +
> +	struct regmap *grf;
> +};
> +
> +#define RK3288_GRF_SOC_CON1 0x0248
> +#define RK3288_GRF_SOC_CON3 0x0250
> +#define RK3288_GRF_GPIO3D_E 0x01ec
> +#define RK3288_GRF_GPIO4A_E 0x01f0
> +#define RK3288_GRF_GPIO4B_E 0x01f4

here you're using a space instead of a tab, please select one pattern either 
tabs or space but do not mix them.


> +#define GPIO3D_2MA	0xFFFF0000
> +#define GPIO3D_4MA	0xFFFF5555
> +#define GPIO3D_8MA	0xFFFFAAAA
> +#define GPIO3D_12MA	0xFFFFFFFF
> +
> +#define GPIO4A_2MA	0xFFFF0000
> +#define GPIO4A_4MA	0xFFFF5555
> +#define GPIO4A_8MA	0xFFFFAAAA
> +#define GPIO4A_12MA	0xFFFFFFFF

see comment about pin settings below


> +
> +#define GRF_BIT(nr)	(BIT(nr) | BIT(nr+16))
> +#define GRF_CLR_BIT(nr)	(BIT(nr+16))
> +
> +#define GPIO4B_2_2MA	(GRF_CLR_BIT(2) | GRF_CLR_BIT(3))
> +#define GPIO4B_2_4MA	(GRF_BIT(2) | GRF_CLR_BIT(3))
> +#define GPIO4B_2_8MA	(GRF_CLR_BIT(2) | GRF_BIT(3))
> +#define GPIO4B_2_12MA	(GRF_BIT(2) | GRF_BIT(3))
> +
> +/*RK3288_GRF_SOC_CON1*/
> +#define GMAC_PHY_INTF_SEL_RGMII	(GRF_BIT(6) | GRF_CLR_BIT(7) |
> GRF_CLR_BIT(8)) 
> +#define GMAC_PHY_INTF_SEL_RMII  (GRF_CLR_BIT(6) |
> GRF_CLR_BIT(7) | GRF_BIT(8)) 
> +#define GMAC_FLOW_CTRL          GRF_BIT(9)
> +#define GMAC_FLOW_CTRL_CLR      GRF_CLR_BIT(9)
> +#define GMAC_SPEED_10M          GRF_CLR_BIT(10)
> +#define GMAC_SPEED_100M         GRF_BIT(10)
> +#define GMAC_RMII_CLK_25M       GRF_BIT(11)
> +#define GMAC_RMII_CLK_2_5M      GRF_CLR_BIT(11)
> +#define GMAC_CLK_125M           (GRF_CLR_BIT(12) | GRF_CLR_BIT(13))
> +#define GMAC_CLK_25M            (GRF_BIT(12) | GRF_BIT(13))
> +#define GMAC_CLK_2_5M           (GRF_CLR_BIT(12) | GRF_BIT(13))
> +#define GMAC_RMII_MODE          GRF_BIT(14)
> +#define GMAC_RMII_MODE_CLR      GRF_CLR_BIT(14)
> +
> +/*RK3288_GRF_SOC_CON3*/
> +#define GMAC_TXCLK_DLY_ENABLE   GRF_BIT(14)
> +#define GMAC_TXCLK_DLY_DISABLE  GRF_CLR_BIT(14)
> +#define GMAC_RXCLK_DLY_ENABLE   GRF_BIT(15)
> +#define GMAC_RXCLK_DLY_DISABLE  GRF_CLR_BIT(15)
> +#define GMAC_CLK_RX_DL_CFG(val) ((0x7F<<7<<16) | (val<<7))
> +#define GMAC_CLK_TX_DL_CFG(val) ((0x7F<<16) | (val))

again mixed tabs and spaces as delimiters.

Also the _CFG macros are not well abstracted. You could take a look at the 
HIWORD_UPDATE macro in drivers/clk/rockchip/clk.h:

#define GMAC_CLK_DL_MASK	0x7f
#define GMAC_CLK_RX_DL_CFG(val)  HIWORD_UPDATE(val, GMAC_CLK_DL_MASK, 7)  
#define GMAC_CLK_TX_DL_CFG(val)  HIWORD_UPDATE(val, GMAC_CLK_DL_MASK, 0)  


> +
> +static void set_to_rgmii(struct rk_priv_data *bsp_priv,
> +			 int tx_delay, int rx_delay)
> +{
> +	if (IS_ERR(bsp_priv->grf)) {
> +		pr_err("%s: Missing rockchip,grf property\n", __func__);
> +		return;
> +	}
> +
> +	regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
> +		     GMAC_PHY_INTF_SEL_RGMII | GMAC_RMII_MODE_CLR);
> +	regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON3,
> +		     GMAC_RXCLK_DLY_ENABLE | GMAC_TXCLK_DLY_ENABLE |
> +		     GMAC_CLK_RX_DL_CFG(rx_delay) |
> +		     GMAC_CLK_TX_DL_CFG(tx_delay));
> +	regmap_write(bsp_priv->grf, RK3288_GRF_GPIO3D_E, GPIO3D_12MA);
> +	regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4A_E, GPIO4A_12MA);
> +	regmap_write(bsp_priv->grf, RK3288_GRF_GPIO4B_E, GPIO4B_2_12MA);

please don't write to parts controlled by other drivers - here the drive 
strength settings of pins is controlled by the pinctrl driver. Instead you can 
just set the drive-strength in the pinctrl settings.


> +
> +	pr_debug("%s: tx delay=0x%x; rx delay=0x%x;\n",
> +		 __func__, tx_delay, rx_delay);
> +}
> +
> +static void set_to_rmii(struct rk_priv_data *bsp_priv)
> +{
> +	if (IS_ERR(bsp_priv->grf)) {
> +		pr_err("%s: Missing rockchip,grf property\n", __func__);

you have a device-reference in rk_priv_data, so you could use dev_err here. 
Same for all other pr_err/pr_debug/pr_* calls in this file.


> +		return;
> +	}
> +
> +	regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
> +		     GMAC_PHY_INTF_SEL_RMII);
> +	regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
> +		     GMAC_RMII_MODE);

these two could be combined?


> +}
> +
> +static void set_rgmii_speed(struct rk_priv_data *bsp_priv, int speed)
> +{
> +	if (IS_ERR(bsp_priv->grf)) {
> +		pr_err("%s: Missing rockchip,grf property\n", __func__);
> +		return;
> +	}
> +
> +	if (speed == 10)
> +		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, GMAC_CLK_2_5M);
> +	else if (speed == 100)
> +		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, GMAC_CLK_25M);
> +	else if (speed == 1000)
> +		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1, GMAC_CLK_125M);
> +	else
> +		pr_err("unknown speed value for RGMII! speed=%d", speed);
> +}
> +
> +static void set_rmii_speed(struct rk_priv_data *bsp_priv, int speed)
> +{
> +	if (IS_ERR(bsp_priv->grf)) {
> +		pr_err("%s: Missing rockchip,grf property\n", __func__);
> +		return;
> +	}
> +
> +	if (speed == 10) {
> +		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
> +			     GMAC_RMII_CLK_2_5M);
> +		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
> +			     GMAC_SPEED_10M);

combine into one write?


> +	} else if (speed == 100) {
> +		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
> +			     GMAC_RMII_CLK_25M);
> +		regmap_write(bsp_priv->grf, RK3288_GRF_SOC_CON1,
> +			     GMAC_SPEED_100M);

combine into one write?


> +	} else {
> +		pr_err("unknown speed value for RMII! speed=%d", speed);
> +	}
> +}
> +
> +#define MAC_CLK_RX	"mac_clk_rx"
> +#define MAC_CLK_TX	"mac_clk_tx"
> +#define CLK_MAC_REF	"clk_mac_ref"
> +#define CLK_MAC_REF_OUT	"clk_mac_refout"
> +#define CLK_MAC_PLL	"clk_mac_pll"
> +#define ACLK_MAC	"aclk_mac"
> +#define PCLK_MAC	"pclk_mac"
> +#define MAC_CLKIN	"ext_gmac"
> +#define CLK_MAC		"stmmaceth"

why the need to extra constants for the clock names and not use the real names 
directly like most other drivers do?


> +
> +static int gmac_clk_init(struct rk_priv_data *bsp_priv)
> +{
> +	struct device *dev = &bsp_priv->pdev->dev;
> +
> +	bsp_priv->clk_enabled = false;
> +
> +	bsp_priv->mac_clk_rx = clk_get(dev, MAC_CLK_RX);
> +	if (IS_ERR(bsp_priv->mac_clk_rx))
> +		pr_warn("%s: warning: cannot get clock %s\n",
> +			__func__, MAC_CLK_RX);
> +
> +	bsp_priv->mac_clk_tx = clk_get(dev, MAC_CLK_TX);
> +	if (IS_ERR(bsp_priv->mac_clk_tx))
> +		pr_warn("%s: warning: cannot get clock %s\n",
> +			__func__, MAC_CLK_TX);
> +
> +	bsp_priv->clk_mac_ref = clk_get(dev, CLK_MAC_REF);
> +	if (IS_ERR(bsp_priv->clk_mac_ref))
> +		pr_warn("%s: warning: cannot get clock %s\n",
> +			__func__, CLK_MAC_REF);
> +
> +	bsp_priv->clk_mac_refout = clk_get(dev, CLK_MAC_REF_OUT);
> +	if (IS_ERR(bsp_priv->clk_mac_refout))
> +		pr_warn("%s: warning:cannot get clock %s\n",
> +			__func__, CLK_MAC_REF_OUT);
> +
> +	bsp_priv->aclk_mac = clk_get(dev, ACLK_MAC);
> +	if (IS_ERR(bsp_priv->aclk_mac))
> +		pr_warn("%s: warning: cannot get clock %s\n",
> +			__func__, ACLK_MAC);
> +
> +	bsp_priv->pclk_mac = clk_get(dev, PCLK_MAC);
> +	if (IS_ERR(bsp_priv->pclk_mac))
> +		pr_warn("%s: warning: cannot get clock %s\n",
> +			__func__, PCLK_MAC);
> +
> +	bsp_priv->clk_mac_pll = clk_get(dev, CLK_MAC_PLL);
> +	if (IS_ERR(bsp_priv->clk_mac_pll))
> +		pr_warn("%s: warning: cannot get clock %s\n",
> +			__func__, CLK_MAC_PLL);
> +
> +	bsp_priv->gmac_clkin = clk_get(dev, MAC_CLKIN);
> +	if (IS_ERR(bsp_priv->gmac_clkin))
> +		pr_warn("%s: warning: cannot get clock %s\n",
> +			__func__, MAC_CLKIN);
> +
> +	bsp_priv->clk_mac = clk_get(dev, CLK_MAC);
> +	if (IS_ERR(bsp_priv->clk_mac))
> +		pr_warn("%s: warning: cannot get clock %s\n",
> +			__func__, CLK_MAC);

there is not clk_put in the _remove case ... maybe you could simply use 
devm_clk_get here so that all clocks are put on device removal.

Also you're warning on every missing clock. Below it looks like you need a 
different set of them for rgmii and rmii, so maybe you should simply error out 
when core clocks for the selected phy-mode are missing.


> +
> +	if (bsp_priv->clock_input) {
> +		pr_info("%s: clock input from PHY\n", __func__);
> +	} else {
> +		if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
> +			clk_set_rate(bsp_priv->clk_mac_pll, 50000000);
> +
> +		clk_set_parent(bsp_priv->clk_mac, bsp_priv->clk_mac_pll);

why the explicit reparenting. The common clock-framework is intelligent enough 
to select the best suitable parent.

In general I'm thinking the clock-handling inside this driver should be 
simplyfied, as the common-clock framework can handle most cases itself. I.e. if 
a 125MHz external clock is present and so on. But haven't looked to deep yet.



> +	}
> +
> +	return 0;
> +}
> +
> +static int gmac_clk_enable(struct rk_priv_data *bsp_priv, bool enable)
> +{
> +	int phy_iface = phy_iface = bsp_priv->phy_iface;
> +
> +	if (enable) {
> +		if (!bsp_priv->clk_enabled) {
> +			if (phy_iface == PHY_INTERFACE_MODE_RMII) {
> +				if (!IS_ERR(bsp_priv->mac_clk_rx))
> +					clk_prepare_enable(
> +						bsp_priv->mac_clk_rx);
> +
> +				if (!IS_ERR(bsp_priv->clk_mac_ref))
> +					clk_prepare_enable(
> +						bsp_priv->clk_mac_ref);
> +
> +				if (!IS_ERR(bsp_priv->clk_mac_refout))
> +					clk_prepare_enable(
> +						bsp_priv->clk_mac_refout);
> +			}
> +
> +			if (!IS_ERR(bsp_priv->aclk_mac))
> +				clk_prepare_enable(bsp_priv->aclk_mac);
> +
> +			if (!IS_ERR(bsp_priv->pclk_mac))
> +				clk_prepare_enable(bsp_priv->pclk_mac);
> +
> +			if (!IS_ERR(bsp_priv->mac_clk_tx))
> +				clk_prepare_enable(bsp_priv->mac_clk_tx);
> +
> +			/**
> +			 * if (!IS_ERR(bsp_priv->clk_mac))
> +			 *	clk_prepare_enable(bsp_priv->clk_mac);
> +			 */
> +			mdelay(5);
> +			bsp_priv->clk_enabled = true;
> +		}
> +	} else {
> +		if (bsp_priv->clk_enabled) {
> +			if (phy_iface == PHY_INTERFACE_MODE_RMII) {
> +				if (!IS_ERR(bsp_priv->mac_clk_rx))
> +					clk_disable_unprepare(
> +						bsp_priv->mac_clk_rx);
> +
> +				if (!IS_ERR(bsp_priv->clk_mac_ref))
> +					clk_disable_unprepare(
> +						bsp_priv->clk_mac_ref);
> +
> +				if (!IS_ERR(bsp_priv->clk_mac_refout))
> +					clk_disable_unprepare(
> +						bsp_priv->clk_mac_refout);
> +			}
> +
> +			if (!IS_ERR(bsp_priv->aclk_mac))
> +				clk_disable_unprepare(bsp_priv->aclk_mac);
> +
> +			if (!IS_ERR(bsp_priv->pclk_mac))
> +				clk_disable_unprepare(bsp_priv->pclk_mac);
> +
> +			if (!IS_ERR(bsp_priv->mac_clk_tx))
> +				clk_disable_unprepare(bsp_priv->mac_clk_tx);
> +			/**
> +			 * if (!IS_ERR(bsp_priv->clk_mac))
> +			 *	clk_disable_unprepare(bsp_priv->clk_mac);
> +			 */
> +			bsp_priv->clk_enabled = false;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int power_on_by_pmu(struct rk_priv_data *bsp_priv, bool enable)
> +{
> +	struct regulator *ldo;
> +	char *ldostr = bsp_priv->pmu_regulator;
> +	int ret;
> +
> +	if (!ldostr) {
> +		pr_err("%s: no ldo found\n", __func__);
> +		return -1;
> +	}
> +
> +	ldo = regulator_get(NULL, ldostr);
> +	if (!ldo) {
> +		pr_err("\n%s get ldo %s failed\n", __func__, ldostr);
> +	} else {
> +		if (enable) {
> +			if (!regulator_is_enabled(ldo)) {
> +				regulator_set_voltage(ldo, 3300000, 3300000);
> +				ret = regulator_enable(ldo);
> +				if (ret != 0)
> +					pr_err("%s: fail to enable %s\n",
> +					       __func__, ldostr);
> +				else
> +					pr_info("turn on ldo done.\n");
> +			} else {
> +				pr_warn("%s is enabled before enable", ldostr);
> +			}
> +		} else {
> +			if (regulator_is_enabled(ldo)) {
> +				ret = regulator_disable(ldo);
> +				if (ret != 0)
> +					pr_err("%s: fail to disable %s\n",
> +					       __func__, ldostr);
> +				else
> +					pr_info("turn off ldo done.\n");
> +			} else {
> +				pr_warn("%s is disabled before disable",
> +					ldostr);
> +			}
> +		}
> +		regulator_put(ldo);
> +	}
> +
> +	return 0;
> +}
> +
> +static int power_on_by_gpio(struct rk_priv_data *bsp_priv, bool enable)
> +{
> +	if (enable) {
> +		/*power on*/
> +		if (gpio_is_valid(bsp_priv->power_io))
> +			gpio_direction_output(bsp_priv->power_io,
> +					      bsp_priv->power_io_level);
> +	} else {
> +		/*power off*/
> +		if (gpio_is_valid(bsp_priv->power_io))
> +			gpio_direction_output(bsp_priv->power_io,
> +					      !bsp_priv->power_io_level);
> +	}
> +
> +	return 0;
> +}
> +
> +static int phy_power_on(struct rk_priv_data *bsp_priv, bool enable)
> +{
> +	int ret = -1;
> +
> +	pr_info("Ethernet PHY power %s\n", enable == 1 ? "on" : "off");
> +
> +	if (bsp_priv->power_ctrl_by_pmu)
> +		ret = power_on_by_pmu(bsp_priv, enable);
> +	else
> +		ret =  power_on_by_gpio(bsp_priv, enable);

this looks wrong. This should always be a regulator. Even a regulator + switch 
controlled by a gpio can still be modelled as regulator, so that you don't 
need this switch and assorted special handling - so just use the regulator API


> +
> +	if (enable) {
> +		/*reset*/
> +		if (gpio_is_valid(bsp_priv->reset_io)) {
> +			gpio_direction_output(bsp_priv->reset_io,
> +					      bsp_priv->reset_io_level);
> +			mdelay(5);
> +			gpio_direction_output(bsp_priv->reset_io,
> +					      !bsp_priv->reset_io_level);
> +		}
> +		mdelay(30);
> +
> +	} else {
> +		/*pull down reset*/
> +		if (gpio_is_valid(bsp_priv->reset_io)) {
> +			gpio_direction_output(bsp_priv->reset_io,
> +					      bsp_priv->reset_io_level);
> +		}
> +	}

I'm not sure yet if it would be better to use the reset framework for this. 
While it says it is also meant for reset-gpios, there does not seem a driver 
for this to exist yet.



> +
> +	return ret;
> +}
> +
> +#define GPIO_PHY_POWER	"gmac_phy_power"
> +#define GPIO_PHY_RESET	"gmac_phy_reset"
> +#define GPIO_PHY_IRQ	"gmac_phy_irq"

again I don't understand why these constants are necessary

> +
> +static void *rk_gmac_setup(struct platform_device *pdev)
> +{
> +	struct rk_priv_data *bsp_priv;
> +	struct device *dev = &pdev->dev;
> +	enum of_gpio_flags flags;
> +	int ret;
> +	const char *strings = NULL;
> +	int value;
> +	int irq;
> +
> +	bsp_priv = devm_kzalloc(dev, sizeof(*bsp_priv), GFP_KERNEL);
> +	if (!bsp_priv)
> +		return ERR_PTR(-ENOMEM);
> +
> +	bsp_priv->phy_iface = of_get_phy_mode(dev->of_node);
> +
> +	ret = of_property_read_string(dev->of_node, "pmu_regulator", &strings);
> +	if (ret) {
> +		pr_err("%s: Can not read property: pmu_regulator.\n", __func__);
> +		bsp_priv->power_ctrl_by_pmu = false;
> +	} else {
> +		pr_info("%s: ethernet phy power controlled by pmu(%s).\n",
> +			__func__, strings);
> +		bsp_priv->power_ctrl_by_pmu = true;
> +		strcpy(bsp_priv->pmu_regulator, strings);
> +	}

There is a generic regulator-dt-binding for regulator-consumers available 
which you should of course use.


> +
> +	ret = of_property_read_u32(dev->of_node, "pmu_enable_level", &value);
> +	if (ret) {
> +		pr_err("%s: Can not read property: pmu_enable_level.\n",
> +		       __func__);
> +		bsp_priv->power_ctrl_by_pmu = false;
> +	} else {
> +		pr_info("%s: PHY power controlled by pmu(level = %s).\n",
> +			__func__, (value == 1) ? "HIGH" : "LOW");
> +		bsp_priv->power_ctrl_by_pmu = true;
> +		bsp_priv->pmu_enable_level = value;
> +	}

What is this used for? Enabling should of course be done via regulator_enable 
and disabling using regulator_disable.


> +
> +	ret = of_property_read_string(dev->of_node, "clock_in_out", &strings);
> +	if (ret) {
> +		pr_err("%s: Can not read property: clock_in_out.\n", __func__);
> +		bsp_priv->clock_input = true;
> +	} else {
> +		pr_info("%s: clock input or output? (%s).\n",
> +			__func__, strings);
> +		if (!strcmp(strings, "input"))
> +			bsp_priv->clock_input = true;
> +		else
> +			bsp_priv->clock_input = false;
> +	}
> +
> +	ret = of_property_read_u32(dev->of_node, "tx_delay", &value);
> +	if (ret) {
> +		bsp_priv->tx_delay = 0x30;
> +		pr_err("%s: Can not read property: tx_delay.", __func__);
> +		pr_err("%s: set tx_delay to 0x%x\n",
> +		       __func__, bsp_priv->tx_delay);
> +	} else {
> +		pr_info("%s: TX delay(0x%x).\n", __func__, value);
> +		bsp_priv->tx_delay = value;
> +	}
> +
> +	ret = of_property_read_u32(dev->of_node, "rx_delay", &value);
> +	if (ret) {
> +		bsp_priv->rx_delay = 0x10;
> +		pr_err("%s: Can not read property: rx_delay.", __func__);
> +		pr_err("%s: set rx_delay to 0x%x\n",
> +		       __func__, bsp_priv->rx_delay);
> +	} else {
> +		pr_info("%s: RX delay(0x%x).\n", __func__, value);
> +		bsp_priv->rx_delay = value;
> +	}
> +
> +	bsp_priv->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
> +							"rockchip,grf");
> +	bsp_priv->phyirq_io =
> +		of_get_named_gpio_flags(dev->of_node,
> +					"phyirq-gpio", 0, &flags);
> +	bsp_priv->phyirq_io_level = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
> +
> +	bsp_priv->reset_io =
> +		of_get_named_gpio_flags(dev->of_node,
> +					"reset-gpio", 0, &flags);
> +	bsp_priv->reset_io_level = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
> +
> +	bsp_priv->power_io =
> +		of_get_named_gpio_flags(dev->of_node, "power-gpio", 0, &flags);
> +	bsp_priv->power_io_level = (flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
> +
> +	/*power*/
> +	if (!gpio_is_valid(bsp_priv->power_io)) {
> +		pr_err("%s: Failed to get GPIO %s.\n",
> +		       __func__, "power-gpio");
> +	} else {
> +		ret = gpio_request(bsp_priv->power_io, GPIO_PHY_POWER);
> +		if (ret)
> +			pr_err("%s: ERROR: Failed to request GPIO %s.\n",
> +			       __func__, GPIO_PHY_POWER);
> +	}

When everything power-related is handled using the regulator api, you don't 
need this


> +
> +	if (!gpio_is_valid(bsp_priv->reset_io)) {
> +		pr_err("%s: ERROR: Get reset-gpio failed.\n", __func__);
> +	} else {
> +		ret = gpio_request(bsp_priv->reset_io, GPIO_PHY_RESET);
> +		if (ret)
> +			pr_err("%s: ERROR: Failed to request GPIO %s.\n",
> +			       __func__, GPIO_PHY_RESET);
> +	}
> +
> +	if (bsp_priv->phyirq_io > 0) {

This is more for my understanding: why does the mac driver need to handle the 
phy interrupt - but I might be overlooking something.


> +		struct plat_stmmacenet_data *plat_dat;
> +
> +		pr_info("PHY irq in use\n");
> +		ret = gpio_request(bsp_priv->phyirq_io, GPIO_PHY_IRQ);
> +		if (ret < 0) {
> +			pr_warn("%s: Failed to request GPIO %s\n",
> +				__func__, GPIO_PHY_IRQ);
> +			goto goon;
> +		}
> +
> +		ret = gpio_direction_input(bsp_priv->phyirq_io);
> +		if (ret < 0) {
> +			pr_err("%s, Failed to set input for GPIO %s\n",
> +			       __func__, GPIO_PHY_IRQ);
> +			gpio_free(bsp_priv->phyirq_io);
> +			goto goon;
> +		}
> +
> +		irq = gpio_to_irq(bsp_priv->phyirq_io);
> +		if (irq < 0) {
> +			ret = irq;
> +			pr_err("Failed to set irq for %s\n",
> +			       GPIO_PHY_IRQ);
> +			gpio_free(bsp_priv->phyirq_io);
> +			goto goon;
> +		}
> +
> +		plat_dat = dev_get_platdata(&pdev->dev);
> +		if (plat_dat)
> +			plat_dat->mdio_bus_data->probed_phy_irq = irq;
> +		else
> +			pr_err("%s: plat_data is NULL\n", __func__);
> +	}
> +
> +goon:
> +	/*rmii or rgmii*/
> +	if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RGMII) {
> +		pr_info("%s: init for RGMII\n", __func__);
> +		set_to_rgmii(bsp_priv, bsp_priv->tx_delay, bsp_priv->rx_delay);
> +	} else if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII) {
> +		pr_info("%s: init for RMII\n", __func__);
> +		set_to_rmii(bsp_priv);
> +	} else {
> +		pr_err("%s: ERROR: NO interface defined!\n", __func__);
> +	}
> +
> +	bsp_priv->pdev = pdev;
> +
> +	gmac_clk_init(bsp_priv);
> +
> +	return bsp_priv;
> +}
> +
> +static int rk_gmac_init(struct platform_device *pdev, void *priv)
> +{
> +	struct rk_priv_data *bsp_priv = priv;
> +	int ret;
> +
> +	ret = phy_power_on(bsp_priv, true);
> +	if (ret)
> +		return ret;
> +
> +	ret = gmac_clk_enable(bsp_priv, true);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static void rk_gmac_exit(struct platform_device *pdev, void *priv)
> +{
> +	struct rk_priv_data *gmac = priv;
> +
> +	phy_power_on(gmac, false);
> +	gmac_clk_enable(gmac, false);
> +}
> +
> +static void rk_fix_speed(void *priv, unsigned int speed)
> +{
> +	struct rk_priv_data *bsp_priv = priv;
> +
> +	if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RGMII)
> +		set_rgmii_speed(bsp_priv, speed);
> +	else if (bsp_priv->phy_iface == PHY_INTERFACE_MODE_RMII)
> +		set_rmii_speed(bsp_priv, speed);
> +	else
> +		pr_err("unsupported interface %d", bsp_priv->phy_iface);
> +}
> +
> +const struct stmmac_of_data rk_gmac_data = {
> +	.has_gmac = 1,
> +	.fix_mac_speed = rk_fix_speed,
> +	.setup = rk_gmac_setup,
> +	.init = rk_gmac_init,
> +	.exit = rk_gmac_exit,
> +};
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c index
> 15814b7..b4dee96 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -33,6 +33,7 @@
> 
>  static const struct of_device_id stmmac_dt_ids[] = {
>  	/* SoC specific glue layers should come before generic bindings */
> +	{ .compatible = "rockchip,rk3288-gmac", .data = &rk_gmac_data},

please name that rk3288_gmac_data [of course the other occurences too] 
It makes it easier to see which soc it is meant for and it's also not save to 
assume that the next one will use the same register + bit positions in the 
grf.


>  	{ .compatible = "amlogic,meson6-dwmac", .data = &meson6_dwmac_data},
>  	{ .compatible = "allwinner,sun7i-a20-gmac", .data = &sun7i_gmac_data},
>  	{ .compatible = "st,stih415-dwmac", .data = &stih4xx_dwmac_data},
> @@ -291,6 +292,8 @@ static int stmmac_pltfr_probe(struct platform_device
> *pdev) return  -ENOMEM;
>  		}
> 
> +		pdev->dev.platform_data = plat_dat;
> +
>  		ret = stmmac_probe_config_dt(pdev, plat_dat, &mac);
>  		if (ret) {
>  			pr_err("%s: main dt probe failed", __func__);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h index
> 25dd1f7..32a0516 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.h
> @@ -24,5 +24,6 @@ extern const struct stmmac_of_data sun7i_gmac_data;
>  extern const struct stmmac_of_data stih4xx_dwmac_data;
>  extern const struct stmmac_of_data stid127_dwmac_data;
>  extern const struct stmmac_of_data socfpga_gmac_data;
> +extern const struct stmmac_of_data rk_gmac_data;
> 
>  #endif /* __STMMAC_PLATFORM_H__ */

^ permalink raw reply

* Re: [PATCHv2 net] i40e: Implement ndo_gso_check()
From: Tom Herbert @ 2014-12-01 23:47 UTC (permalink / raw)
  To: Joe Stringer
  Cc: Jesse Gross, netdev, Shannon Nelson, Brandeburg, Jesse,
	Jeff Kirsher, linux.nics, Linux Kernel Mailing List
In-Reply-To: <CANr6G5xRbG=UnJEdBEN_5ZRjNikCjuUseb1VbQmjjfb36jDP0g@mail.gmail.com>

On Mon, Dec 1, 2014 at 3:35 PM, Joe Stringer <joestringer@nicira.com> wrote:
> On 21 November 2014 at 09:59, Joe Stringer <joestringer@nicira.com> wrote:
>> On 20 November 2014 16:19, Jesse Gross <jesse@nicira.com> wrote:
>>> I don't know if we need to have the check at all for IPIP though -
>>> after all the driver doesn't expose support for it all (actually it
>>> doesn't expose GRE either). This raises kind of an interesting
>>> question about the checks though - it's pretty easy to add support to
>>> the driver for a new GSO type (and I imagine that people will be
>>> adding GRE soon) and forget to update the check.
>>
>> If the check is more conservative, then testing would show that it's
>> not working and lead people to figure out why (and update the check).
>
> More concretely, one suggestion would be something like following at
> the start of each gso_check():
>
> +       const int supported = SKB_GSO_TCPV4 | SKB_GSO_TCPV6 | SKB_GSO_FCOE |
> +                             SKB_GSO_UDP | SKB_GSO_UDP_TUNNEL;
> +
> +       if (skb_shinfo(skb)->gso_type & ~supported)
> +               return false;

This should already be handled by net_gso_ok.

>
> ..followed by checking the specifics for each. So far the patches have
> only been concerned with further checking on UDP tunnels.

^ permalink raw reply

* Re: [PATCHv2 net] i40e: Implement ndo_gso_check()
From: Jesse Gross @ 2014-12-01 23:53 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Joe Stringer, netdev, Shannon Nelson, Brandeburg, Jesse,
	Jeff Kirsher, linux.nics, Linux Kernel Mailing List
In-Reply-To: <CA+mtBx-Q=Ebrx_8NVB5mWURj-oWB3gdNs5fSAL=Frr9sOmyvHw@mail.gmail.com>

On Mon, Dec 1, 2014 at 3:47 PM, Tom Herbert <therbert@google.com> wrote:
> On Mon, Dec 1, 2014 at 3:35 PM, Joe Stringer <joestringer@nicira.com> wrote:
>> On 21 November 2014 at 09:59, Joe Stringer <joestringer@nicira.com> wrote:
>>> On 20 November 2014 16:19, Jesse Gross <jesse@nicira.com> wrote:
>>>> I don't know if we need to have the check at all for IPIP though -
>>>> after all the driver doesn't expose support for it all (actually it
>>>> doesn't expose GRE either). This raises kind of an interesting
>>>> question about the checks though - it's pretty easy to add support to
>>>> the driver for a new GSO type (and I imagine that people will be
>>>> adding GRE soon) and forget to update the check.
>>>
>>> If the check is more conservative, then testing would show that it's
>>> not working and lead people to figure out why (and update the check).
>>
>> More concretely, one suggestion would be something like following at
>> the start of each gso_check():
>>
>> +       const int supported = SKB_GSO_TCPV4 | SKB_GSO_TCPV6 | SKB_GSO_FCOE |
>> +                             SKB_GSO_UDP | SKB_GSO_UDP_TUNNEL;
>> +
>> +       if (skb_shinfo(skb)->gso_type & ~supported)
>> +               return false;
>
> This should already be handled by net_gso_ok.

My original point wasn't so much that this isn't handled at the moment
but that it's easy to add a supported GSO type but then forget to
update this check - i.e. if a driver already supports UDP_TUNNEL and
adds support for GRE with the same constraints. It seems not entirely
ideal that this function is acting as a blacklist rather than a
whitelist.

^ 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