| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 14 Jun 2015 11:00:11 -0700
From: Scott Feldman <sfeldma@...il.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: Netdev <netdev@...r.kernel.org>,
"simon.horman@...ronome.com" <simon.horman@...ronome.com>,
Roopa Prabhu <roopa@...ulusnetworks.com>,
"Arad, Ronen" <ronen.arad@...el.com>,
"Fastabend, John R" <john.r.fastabend@...el.com>,
"andrew@...n.ch" <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>,
Guenter Roeck <linux@...ck-us.net>,
davidch <davidch@...adcom.com>,
"stephen@...workplumber.org" <stephen@...workplumber.org>
Subject: Re: [RFC PATCH net-next 3/4] rocker: add fwd_mark support
On Sun, Jun 14, 2015 at 12:02 AM, Jiri Pirko <jiri@...nulli.us> wrote:
> Sat, Jun 13, 2015 at 08:04:29PM CEST, sfeldma@...il.com wrote:
>>From: Scott Feldman <sfeldma@...il.com>
>>
>>If device flags ingress packet as "fwd offload", mark the skb->fwd_mark
>>using the ingress port's dev->fwd_mark. This will be the hint to the
>>kernel that this packet has already been forwarded by device to egress
>>ports matching skb->fwd_mark.
>>
>>For rocker, derive port dev->fwd_mark based on device switch ID. If port
>>is bridged, include the bridge's ifindex in the key for deriving
>>dev->fwd_mark.
>>
>>Signed-off-by: Scott Feldman <sfeldma@...il.com>
>>---
>> drivers/net/ethernet/rocker/rocker.c | 24 ++++++++++++++++++++++++
>> drivers/net/ethernet/rocker/rocker.h | 1 +
>> 2 files changed, 25 insertions(+)
>>
>>diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
>>index a06b93d..81407d8 100644
>>--- a/drivers/net/ethernet/rocker/rocker.c
>>+++ b/drivers/net/ethernet/rocker/rocker.c
>>@@ -4701,6 +4701,7 @@ static int rocker_port_rx_proc(const struct rocker *rocker,
>> const struct rocker_tlv *attrs[ROCKER_TLV_RX_MAX + 1];
>> struct sk_buff *skb = rocker_desc_cookie_ptr_get(desc_info);
>> size_t rx_len;
>>+ u16 rx_flags = 0;
>>
>> if (!skb)
>> return -ENOENT;
>>@@ -4708,6 +4709,8 @@ static int rocker_port_rx_proc(const struct rocker *rocker,
>> rocker_tlv_parse_desc(attrs, ROCKER_TLV_RX_MAX, desc_info);
>> if (!attrs[ROCKER_TLV_RX_FRAG_LEN])
>> return -EINVAL;
>>+ if (attrs[ROCKER_TLV_RX_FLAGS])
>>+ rx_flags = rocker_tlv_get_u16(attrs[ROCKER_TLV_RX_FLAGS]);
>>
>> rocker_dma_rx_ring_skb_unmap(rocker, attrs);
>>
>>@@ -4715,6 +4718,9 @@ static int rocker_port_rx_proc(const struct rocker *rocker,
>> skb_put(skb, rx_len);
>> skb->protocol = eth_type_trans(skb, rocker_port->dev);
>>
>>+ if (rx_flags & ROCKER_RX_FLAGS_FWD_OFFLOAD)
>
> Nasty :) I'm not sure that this can be easily done for real hw. But
> anyway, that is a driver problem.
Why is this nasty?
Some real HW can do it, some can't. Some that can go into detail of
the nature of the fwd. Rocker can do it because we know when we copy
a pkt to the CPU port that's also been forwarded.
>
>>+ skb->fwd_mark = rocker_port->dev->fwd_mark;
>>+
>> rocker_port->dev->stats.rx_packets++;
>> rocker_port->dev->stats.rx_bytes += skb->len;
>>
>>@@ -4814,6 +4820,21 @@ static void rocker_port_dev_addr_init(struct rocker_port *rocker_port)
>> }
>> }
>>
>>+static void rocker_port_fwd_mark_set(struct rocker_port *rocker_port)
>>+{
>>+ struct rocker *rocker = rocker_port->rocker;
>>+ struct {
>>+ u64 hw_id;
>>+ int ifindex;
>>+ } key = {
>>+ .hw_id = rocker->hw.id,
>>+ .ifindex = rocker_port_is_bridged(rocker_port) ?
>>+ rocker_port->bridge_dev->ifindex : 0,
>>+ };
>>+
>>+ rocker_port->dev->fwd_mark = switchdev_mark_get(&key, sizeof(key));
>>+}
>>+
>> static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
>> {
>> const struct pci_dev *pdev = rocker->pdev;
>>@@ -4832,6 +4853,7 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
>> rocker_port->pport = port_number + 1;
>> rocker_port->brport_flags = BR_LEARNING | BR_LEARNING_SYNC;
>> INIT_LIST_HEAD(&rocker_port->trans_mem);
>>+ rocker_port_fwd_mark_set(rocker_port);
>>
>> rocker_port_dev_addr_init(rocker_port);
>> dev->netdev_ops = &rocker_port_netdev_ops;
>>@@ -5131,6 +5153,7 @@ static int rocker_port_bridge_join(struct rocker_port *rocker_port,
>> rocker_port_internal_vlan_id_get(rocker_port, bridge->ifindex);
>>
>> rocker_port->bridge_dev = bridge;
>>+ rocker_port_fwd_mark_set(rocker_port);
>>
>> return rocker_port_vlan_add(rocker_port, SWITCHDEV_TRANS_NONE,
>> untagged_vid, 0);
>>@@ -5152,6 +5175,7 @@ static int rocker_port_bridge_leave(struct rocker_port *rocker_port)
>> rocker_port->dev->ifindex);
>>
>> rocker_port->bridge_dev = NULL;
>>+ rocker_port_fwd_mark_set(rocker_port);
>
>
> Hmm, wouldn't it make sense to move fwd_mark setting from drivers to
> switchdev core? This will look the same for all drivers, so I think it
> should be in common swithdev core, always generated according to "switch id".
This is setting the fwd_mark on the port during init, not the skb
during run-time.
> So all what driver needs to do is to call some helper like:
> static inline void switchdev_skb_fwd_mark(struct sk_buff *skb,
> struct net_device *dev) {
> skb->fwd_mark = dev->fwd_mark;
> }
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists