* [PATCH net-next 01/11] s390/qeth: fix race in used-buffer accounting
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>
By updating q->used_buffers only _after_ do_QDIO() has completed, there
is a potential race against the buffer's TX completion. In the unlikely
case that the TX completion path wins, qeth_qdio_output_handler() would
decrement the counter before qeth_flush_buffers() even incremented it.
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index d80972b9bfc7..f7ddd6455638 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -3506,13 +3506,14 @@ static void qeth_flush_buffers(struct qeth_qdio_out_q *queue, int index,
qdio_flags = QDIO_FLAG_SYNC_OUTPUT;
if (atomic_read(&queue->set_pci_flags_count))
qdio_flags |= QDIO_FLAG_PCI_OUT;
+ atomic_add(count, &queue->used_buffers);
+
rc = do_QDIO(CARD_DDEV(queue->card), qdio_flags,
queue->queue_no, index, count);
if (queue->card->options.performance_stats)
queue->card->perf_stats.outbound_do_qdio_time +=
qeth_get_micros() -
queue->card->perf_stats.outbound_do_qdio_start_time;
- atomic_add(count, &queue->used_buffers);
if (rc) {
queue->card->stats.tx_errors += count;
/* ignore temporary SIGA errors without busy condition */
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 09/11] s390/qeth: merge linearize-check into HW header construction
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>
When checking whether an skb needs to be linearized to fit into an IO
buffer, it's desirable to consider the skb's final size and layout
(ie. after the HW header was added). But a subsequent linearization can
then cause the re-positioned HW header to violate its alignment
restrictions.
Dealing with this situation in two different code paths is quite tricky.
This patch integrates a) linearize-check and b) HW header construction
into one 3 step-sequence:
1. evaluate how the HW header needs to be added (to identify if it takes
up an additional buffer element), then
2. check if the required buffer elements exceed the device's limit.
Linearize when necessary and re-evaluate the HW header placement.
3. Add the HW header in the best-possible way:
a) push, without taking up an additional buffer element
b) push, but consume another buffer element
c) allocate a header object from the cache.
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
drivers/s390/net/qeth_core.h | 4 +-
drivers/s390/net/qeth_core_main.c | 86 ++++++++++++++++++++++++++++++++-------
drivers/s390/net/qeth_l2_main.c | 29 +------------
drivers/s390/net/qeth_l3_main.c | 31 ++------------
4 files changed, 80 insertions(+), 70 deletions(-)
diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 6d8005af67f5..2a5ec99643df 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -1047,7 +1047,9 @@ netdev_features_t qeth_features_check(struct sk_buff *skb,
struct net_device *dev,
netdev_features_t features);
int qeth_vm_request_mac(struct qeth_card *card);
-int qeth_push_hdr(struct sk_buff *skb, struct qeth_hdr **hdr, unsigned int len);
+int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
+ struct qeth_hdr **hdr, unsigned int len,
+ unsigned int *elements);
/* exports for OSN */
int qeth_osn_assist(struct net_device *, void *, int);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 84f1e1e33f3f..e7b34624df1e 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -3831,6 +3831,17 @@ int qeth_get_elements_for_frags(struct sk_buff *skb)
}
EXPORT_SYMBOL_GPL(qeth_get_elements_for_frags);
+static unsigned int qeth_count_elements(struct sk_buff *skb, int data_offset)
+{
+ unsigned int elements = qeth_get_elements_for_frags(skb);
+ addr_t end = (addr_t)skb->data + skb_headlen(skb);
+ addr_t start = (addr_t)skb->data + data_offset;
+
+ if (start != end)
+ elements += qeth_get_elements_for_range(start, end);
+ return elements;
+}
+
/**
* qeth_get_elements_no() - find number of SBALEs for skb data, inc. frags.
* @card: qeth card structure, to check max. elems.
@@ -3846,12 +3857,7 @@ EXPORT_SYMBOL_GPL(qeth_get_elements_for_frags);
int qeth_get_elements_no(struct qeth_card *card,
struct sk_buff *skb, int extra_elems, int data_offset)
{
- addr_t end = (addr_t)skb->data + skb_headlen(skb);
- int elements = qeth_get_elements_for_frags(skb);
- addr_t start = (addr_t)skb->data + data_offset;
-
- if (start != end)
- elements += qeth_get_elements_for_range(start, end);
+ int elements = qeth_count_elements(skb, data_offset);
if ((elements + extra_elems) > QETH_MAX_BUFFER_ELEMENTS(card)) {
QETH_DBF_MESSAGE(2, "Invalid size of IP packet "
@@ -3885,22 +3891,72 @@ int qeth_hdr_chk_and_bounce(struct sk_buff *skb, struct qeth_hdr **hdr, int len)
EXPORT_SYMBOL_GPL(qeth_hdr_chk_and_bounce);
/**
- * qeth_push_hdr() - push a qeth_hdr onto an skb.
- * @skb: skb that the qeth_hdr should be pushed onto.
+ * qeth_add_hw_header() - add a HW header to an skb.
+ * @skb: skb that the HW header should be added to.
* @hdr: double pointer to a qeth_hdr. When returning with >= 0,
* it contains a valid pointer to a qeth_hdr.
- * @len: length of the hdr that needs to be pushed on.
+ * @len: length of the HW header.
*
* Returns the pushed length. If the header can't be pushed on
* (eg. because it would cross a page boundary), it is allocated from
* the cache instead and 0 is returned.
+ * The number of needed buffer elements is returned in @elements.
* Error to create the hdr is indicated by returning with < 0.
*/
-int qeth_push_hdr(struct sk_buff *skb, struct qeth_hdr **hdr, unsigned int len)
-{
- if (skb_headroom(skb) >= len &&
- qeth_get_elements_for_range((addr_t)skb->data - len,
- (addr_t)skb->data) == 1) {
+int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
+ struct qeth_hdr **hdr, unsigned int len,
+ unsigned int *elements)
+{
+ const unsigned int max_elements = QETH_MAX_BUFFER_ELEMENTS(card);
+ unsigned int __elements;
+ addr_t start, end;
+ bool push_ok;
+ int rc;
+
+check_layout:
+ start = (addr_t)skb->data - len;
+ end = (addr_t)skb->data;
+
+ if (qeth_get_elements_for_range(start, end + 1) == 1) {
+ /* Push HW header into same page as first protocol header. */
+ push_ok = true;
+ __elements = qeth_count_elements(skb, 0);
+ } else {
+ __elements = 1 + qeth_count_elements(skb, 0);
+ if (qeth_get_elements_for_range(start, end) == 1)
+ /* Push HW header into a new page. */
+ push_ok = true;
+ else
+ /* Use header cache. */
+ push_ok = false;
+ }
+
+ /* Compress skb to fit into one IO buffer: */
+ if (__elements > max_elements) {
+ if (!skb_is_nonlinear(skb)) {
+ /* Drop it, no easy way of shrinking it further. */
+ QETH_DBF_MESSAGE(2, "Dropped an oversized skb (Max Elements=%u / Actual=%u / Length=%u).\n",
+ max_elements, __elements, skb->len);
+ return -E2BIG;
+ }
+
+ rc = skb_linearize(skb);
+ if (card->options.performance_stats) {
+ if (rc)
+ card->perf_stats.tx_linfail++;
+ else
+ card->perf_stats.tx_lin++;
+ }
+ if (rc)
+ return rc;
+
+ /* Linearization changed the layout, re-evaluate: */
+ goto check_layout;
+ }
+
+ *elements = __elements;
+ /* Add the header: */
+ if (push_ok) {
*hdr = skb_push(skb, len);
return len;
}
@@ -3910,7 +3966,7 @@ int qeth_push_hdr(struct sk_buff *skb, struct qeth_hdr **hdr, unsigned int len)
return -ENOMEM;
return 0;
}
-EXPORT_SYMBOL_GPL(qeth_push_hdr);
+EXPORT_SYMBOL_GPL(qeth_add_hw_header);
static void __qeth_fill_buffer(struct sk_buff *skb,
struct qeth_qdio_out_buffer *buf,
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index a785c5ff73cd..905f3bb3a87c 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -672,39 +672,21 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
int ipv)
{
int push_len = sizeof(struct qeth_hdr);
- unsigned int hdr_elements = 0;
struct qeth_hdr *hdr = NULL;
unsigned int hd_len = 0;
unsigned int elements;
bool is_sg;
int rc;
- /* fix hardware limitation: as long as we do not have sbal
- * chaining we can not send long frag lists
- */
- if (!qeth_get_elements_no(card, skb, 0, 0)) {
- rc = skb_linearize(skb);
-
- if (card->options.performance_stats) {
- if (rc)
- card->perf_stats.tx_linfail++;
- else
- card->perf_stats.tx_lin++;
- }
- if (rc)
- return rc;
- }
-
rc = skb_cow_head(skb, push_len);
if (rc)
return rc;
- push_len = qeth_push_hdr(skb, &hdr, push_len);
+ push_len = qeth_add_hw_header(card, skb, &hdr, push_len, &elements);
if (push_len < 0)
return push_len;
if (!push_len) {
/* hdr was allocated from cache */
hd_len = sizeof(*hdr);
- hdr_elements = 1;
}
qeth_l2_fill_header(hdr, skb, cast_type, skb->len - push_len);
if (skb->ip_summed == CHECKSUM_PARTIAL) {
@@ -713,18 +695,11 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
card->perf_stats.tx_csum++;
}
- elements = qeth_get_elements_no(card, skb, hdr_elements, 0);
- if (!elements) {
- rc = -E2BIG;
- goto out;
- }
- elements += hdr_elements;
-
is_sg = skb_is_nonlinear(skb);
/* TODO: remove the skb_orphan() once TX completion is fast enough */
skb_orphan(skb);
rc = qeth_do_send_packet(card, queue, skb, hdr, 0, hd_len, elements);
-out:
+
if (!rc) {
if (card->options.performance_stats) {
card->perf_stats.buf_elements_sent += elements;
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index c12aeb7d8f26..f7bcc4853c45 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2166,28 +2166,13 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
int cast_type)
{
const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
+ unsigned int frame_len, elements;
unsigned char eth_hdr[ETH_HLEN];
- unsigned int hdr_elements = 0;
struct qeth_hdr *hdr = NULL;
- int elements, push_len, rc;
unsigned int hd_len = 0;
- unsigned int frame_len;
+ int push_len, rc;
bool is_sg;
- /* compress skb to fit into one IO buffer: */
- if (!qeth_get_elements_no(card, skb, 0, 0)) {
- rc = skb_linearize(skb);
-
- if (card->options.performance_stats) {
- if (rc)
- card->perf_stats.tx_linfail++;
- else
- card->perf_stats.tx_lin++;
- }
- if (rc)
- return rc;
- }
-
/* re-use the L2 header area for the HW header: */
rc = skb_cow_head(skb, hw_hdr_len - ETH_HLEN);
if (rc)
@@ -2196,22 +2181,14 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
skb_pull(skb, ETH_HLEN);
frame_len = skb->len;
- push_len = qeth_push_hdr(skb, &hdr, hw_hdr_len);
+ push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, &elements);
if (push_len < 0)
return push_len;
if (!push_len) {
/* hdr was added discontiguous from skb->data */
hd_len = hw_hdr_len;
- hdr_elements = 1;
}
- elements = qeth_get_elements_no(card, skb, hdr_elements, 0);
- if (!elements) {
- rc = -E2BIG;
- goto out;
- }
- elements += hdr_elements;
-
if (skb->protocol == htons(ETH_P_AF_IUCV))
qeth_l3_fill_af_iucv_hdr(hdr, skb, frame_len);
else
@@ -2226,7 +2203,7 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
rc = qeth_do_send_packet(card, queue, skb, hdr, 0, hd_len,
elements);
}
-out:
+
if (!rc) {
if (card->options.performance_stats) {
card->perf_stats.buf_elements_sent += elements;
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 10/11] s390/qeth: add support for constrained HW headers
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>
Some transmit modes require that the HW header is located in the same
page as the initial protocol headers in skb->data. Let callers specify
the size of this contiguous header range, and enforce it when building
the HW header.
While at it, apply some gentle renaming to the relevant L2 code so that
it matches the L3 code.
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
drivers/s390/net/qeth_core.h | 4 ++--
drivers/s390/net/qeth_core_main.c | 33 +++++++++++++++++++--------------
drivers/s390/net/qeth_l2_main.c | 12 +++++++-----
drivers/s390/net/qeth_l3_main.c | 3 ++-
4 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 2a5ec99643df..605ec4706773 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -1048,8 +1048,8 @@ netdev_features_t qeth_features_check(struct sk_buff *skb,
netdev_features_t features);
int qeth_vm_request_mac(struct qeth_card *card);
int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
- struct qeth_hdr **hdr, unsigned int len,
- unsigned int *elements);
+ struct qeth_hdr **hdr, unsigned int hdr_len,
+ unsigned int proto_len, unsigned int *elements);
/* exports for OSN */
int qeth_osn_assist(struct net_device *, void *, int);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index e7b34624df1e..732b517369c7 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -3895,7 +3895,9 @@ EXPORT_SYMBOL_GPL(qeth_hdr_chk_and_bounce);
* @skb: skb that the HW header should be added to.
* @hdr: double pointer to a qeth_hdr. When returning with >= 0,
* it contains a valid pointer to a qeth_hdr.
- * @len: length of the HW header.
+ * @hdr_len: length of the HW header.
+ * @proto_len: length of protocol headers that need to be in same page as the
+ * HW header.
*
* Returns the pushed length. If the header can't be pushed on
* (eg. because it would cross a page boundary), it is allocated from
@@ -3904,31 +3906,32 @@ EXPORT_SYMBOL_GPL(qeth_hdr_chk_and_bounce);
* Error to create the hdr is indicated by returning with < 0.
*/
int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
- struct qeth_hdr **hdr, unsigned int len,
- unsigned int *elements)
+ struct qeth_hdr **hdr, unsigned int hdr_len,
+ unsigned int proto_len, unsigned int *elements)
{
const unsigned int max_elements = QETH_MAX_BUFFER_ELEMENTS(card);
+ const unsigned int contiguous = proto_len ? proto_len : 1;
unsigned int __elements;
addr_t start, end;
bool push_ok;
int rc;
check_layout:
- start = (addr_t)skb->data - len;
+ start = (addr_t)skb->data - hdr_len;
end = (addr_t)skb->data;
- if (qeth_get_elements_for_range(start, end + 1) == 1) {
+ if (qeth_get_elements_for_range(start, end + contiguous) == 1) {
/* Push HW header into same page as first protocol header. */
push_ok = true;
__elements = qeth_count_elements(skb, 0);
- } else {
+ } else if (!proto_len && qeth_get_elements_for_range(start, end) == 1) {
+ /* Push HW header into a new page. */
+ push_ok = true;
__elements = 1 + qeth_count_elements(skb, 0);
- if (qeth_get_elements_for_range(start, end) == 1)
- /* Push HW header into a new page. */
- push_ok = true;
- else
- /* Use header cache. */
- push_ok = false;
+ } else {
+ /* Use header cache, copy protocol headers up. */
+ push_ok = false;
+ __elements = 1 + qeth_count_elements(skb, proto_len);
}
/* Compress skb to fit into one IO buffer: */
@@ -3957,13 +3960,15 @@ int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
*elements = __elements;
/* Add the header: */
if (push_ok) {
- *hdr = skb_push(skb, len);
- return len;
+ *hdr = skb_push(skb, hdr_len);
+ return hdr_len;
}
/* fall back */
*hdr = kmem_cache_alloc(qeth_core_header_cache, GFP_ATOMIC);
if (!*hdr)
return -ENOMEM;
+ /* Copy protocol headers behind HW header: */
+ skb_copy_from_linear_data(skb, ((char *)*hdr) + hdr_len, proto_len);
return 0;
}
EXPORT_SYMBOL_GPL(qeth_add_hw_header);
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 905f3bb3a87c..c302002e422f 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -671,17 +671,19 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
struct qeth_qdio_out_q *queue, int cast_type,
int ipv)
{
- int push_len = sizeof(struct qeth_hdr);
+ const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
struct qeth_hdr *hdr = NULL;
unsigned int hd_len = 0;
unsigned int elements;
+ int push_len, rc;
bool is_sg;
- int rc;
- rc = skb_cow_head(skb, push_len);
+ rc = skb_cow_head(skb, hw_hdr_len);
if (rc)
return rc;
- push_len = qeth_add_hw_header(card, skb, &hdr, push_len, &elements);
+
+ push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, 0,
+ &elements);
if (push_len < 0)
return push_len;
if (!push_len) {
@@ -707,7 +709,7 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
card->perf_stats.sg_skbs_sent++;
}
} else {
- if (hd_len)
+ if (!push_len)
kmem_cache_free(qeth_core_header_cache, hdr);
if (rc == -EBUSY)
/* roll back to ETH header */
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index f7bcc4853c45..b8e828556cf6 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2181,7 +2181,8 @@ static int qeth_l3_xmit_offload(struct qeth_card *card, struct sk_buff *skb,
skb_pull(skb, ETH_HLEN);
frame_len = skb->len;
- push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, &elements);
+ push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, 0,
+ &elements);
if (push_len < 0)
return push_len;
if (!push_len) {
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 11/11] s390/qeth: speed up L2 IQD xmit
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>
Modify the L2 OSA xmit path so that it also supports L2 IQD devices
(in particular, their HW header requirements). This allows IQD devices
to advertise NETIF_F_SG support, and eliminates the allocation overhead
for the HW header.
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 7 ++++
drivers/s390/net/qeth_l2_main.c | 76 ++++++++++++---------------------------
drivers/s390/net/qeth_l3_main.c | 3 --
3 files changed, 30 insertions(+), 56 deletions(-)
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 732b517369c7..d09a7110b381 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -5731,6 +5731,13 @@ static struct net_device *qeth_alloc_netdev(struct qeth_card *card)
dev->mtu = 0;
SET_NETDEV_DEV(dev, &card->gdev->dev);
netif_carrier_off(dev);
+
+ if (!IS_OSN(card)) {
+ dev->priv_flags &= ~IFF_TX_SKB_SHARING;
+ dev->hw_features |= NETIF_F_SG;
+ dev->vlan_features |= NETIF_F_SG;
+ }
+
return dev;
}
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index c302002e422f..c1829a4b955d 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -641,37 +641,13 @@ static void qeth_l2_set_rx_mode(struct net_device *dev)
qeth_promisc_to_bridge(card);
}
-static int qeth_l2_xmit_iqd(struct qeth_card *card, struct sk_buff *skb,
- struct qeth_qdio_out_q *queue, int cast_type)
-{
- unsigned int data_offset = ETH_HLEN;
- struct qeth_hdr *hdr;
- int rc;
-
- hdr = kmem_cache_alloc(qeth_core_header_cache, GFP_ATOMIC);
- if (!hdr)
- return -ENOMEM;
- qeth_l2_fill_header(hdr, skb, cast_type, skb->len);
- skb_copy_from_linear_data(skb, ((char *)hdr) + sizeof(*hdr),
- data_offset);
-
- if (!qeth_get_elements_no(card, skb, 1, data_offset)) {
- rc = -E2BIG;
- goto out;
- }
- rc = qeth_do_send_packet_fast(queue, skb, hdr, data_offset,
- sizeof(*hdr) + data_offset);
-out:
- if (rc)
- kmem_cache_free(qeth_core_header_cache, hdr);
- return rc;
-}
-
-static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
- struct qeth_qdio_out_q *queue, int cast_type,
- int ipv)
+static int qeth_l2_xmit(struct qeth_card *card, struct sk_buff *skb,
+ struct qeth_qdio_out_q *queue, int cast_type, int ipv)
{
+ const unsigned int proto_len = IS_IQD(card) ? ETH_HLEN : 0;
const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
+ unsigned int frame_len = skb->len;
+ unsigned int data_offset = 0;
struct qeth_hdr *hdr = NULL;
unsigned int hd_len = 0;
unsigned int elements;
@@ -682,15 +658,16 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
if (rc)
return rc;
- push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, 0,
+ push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, proto_len,
&elements);
if (push_len < 0)
return push_len;
if (!push_len) {
- /* hdr was allocated from cache */
- hd_len = sizeof(*hdr);
+ /* HW header needs its own buffer element. */
+ hd_len = hw_hdr_len + proto_len;
+ data_offset = proto_len;
}
- qeth_l2_fill_header(hdr, skb, cast_type, skb->len - push_len);
+ qeth_l2_fill_header(hdr, skb, cast_type, frame_len);
if (skb->ip_summed == CHECKSUM_PARTIAL) {
qeth_tx_csum(skb, &hdr->hdr.l2.flags[1], ipv);
if (card->options.performance_stats)
@@ -698,9 +675,15 @@ static int qeth_l2_xmit_osa(struct qeth_card *card, struct sk_buff *skb,
}
is_sg = skb_is_nonlinear(skb);
- /* TODO: remove the skb_orphan() once TX completion is fast enough */
- skb_orphan(skb);
- rc = qeth_do_send_packet(card, queue, skb, hdr, 0, hd_len, elements);
+ if (IS_IQD(card)) {
+ rc = qeth_do_send_packet_fast(queue, skb, hdr, data_offset,
+ hd_len);
+ } else {
+ /* TODO: drop skb_orphan() once TX completion is fast enough */
+ skb_orphan(skb);
+ rc = qeth_do_send_packet(card, queue, skb, hdr, data_offset,
+ hd_len, elements);
+ }
if (!rc) {
if (card->options.performance_stats) {
@@ -759,16 +742,10 @@ static netdev_tx_t qeth_l2_hard_start_xmit(struct sk_buff *skb,
}
netif_stop_queue(dev);
- switch (card->info.type) {
- case QETH_CARD_TYPE_OSN:
+ if (IS_OSN(card))
rc = qeth_l2_xmit_osn(card, skb, queue);
- break;
- case QETH_CARD_TYPE_IQD:
- rc = qeth_l2_xmit_iqd(card, skb, queue, cast_type);
- break;
- default:
- rc = qeth_l2_xmit_osa(card, skb, queue, cast_type, ipv);
- }
+ else
+ rc = qeth_l2_xmit(card, skb, queue, cast_type, ipv);
if (!rc) {
card->stats.tx_packets++;
@@ -927,6 +904,7 @@ static int qeth_l2_setup_netdev(struct qeth_card *card)
card->dev->flags |= IFF_NOARP;
} else {
card->dev->ethtool_ops = &qeth_l2_ethtool_ops;
+ card->dev->needed_headroom = sizeof(struct qeth_hdr);
}
if (card->info.type == QETH_CARD_TYPE_OSM)
@@ -934,14 +912,6 @@ static int qeth_l2_setup_netdev(struct qeth_card *card)
else
card->dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
- if (card->info.type != QETH_CARD_TYPE_OSN &&
- card->info.type != QETH_CARD_TYPE_IQD) {
- card->dev->priv_flags &= ~IFF_TX_SKB_SHARING;
- card->dev->needed_headroom = sizeof(struct qeth_hdr);
- card->dev->hw_features |= NETIF_F_SG;
- card->dev->vlan_features |= NETIF_F_SG;
- }
-
if (card->info.type == QETH_CARD_TYPE_OSD && !card->info.guestlan) {
card->dev->features |= NETIF_F_SG;
/* OSA 3S and earlier has no RX/TX support */
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index b8e828556cf6..1833e7505aca 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2556,13 +2556,10 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
return -ENODEV;
card->dev->ethtool_ops = &qeth_l3_ethtool_ops;
- card->dev->priv_flags &= ~IFF_TX_SKB_SHARING;
card->dev->needed_headroom = sizeof(struct qeth_hdr) - ETH_HLEN;
card->dev->features |= NETIF_F_HW_VLAN_CTAG_TX |
NETIF_F_HW_VLAN_CTAG_RX |
NETIF_F_HW_VLAN_CTAG_FILTER;
- card->dev->hw_features |= NETIF_F_SG;
- card->dev->vlan_features |= NETIF_F_SG;
netif_keep_dst(card->dev);
if (card->dev->hw_features & NETIF_F_TSO)
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 00/11] s390/qeth: updates 2018-07-19
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
Hi Dave,
please apply one more round of qeth patches to net-next.
This brings additional performance improvements for the transmit code,
and some refactoring to pave the way for using netdev_priv.
Also, two minor fixes for rare corner cases.
Thanks,
Julian
Julian Wiedmann (11):
s390/qeth: fix race in used-buffer accounting
s390/qeth: reset layer2 attribute on layer switch
s390/qeth: remove redundant netif_carrier_ok() checks
s390/qeth: allocate netdevice early
s390/qeth: don't cache HW port number
s390/qeth: simplify max MTU handling
s390/qeth: use core MTU range checking
s390/qeth: add statistics for consumed buffer elements
s390/qeth: merge linearize-check into HW header construction
s390/qeth: add support for constrained HW headers
s390/qeth: speed up L2 IQD xmit
drivers/s390/net/qeth_core.h | 11 +-
drivers/s390/net/qeth_core_main.c | 283 ++++++++++++++++++++++++--------------
drivers/s390/net/qeth_core_mpc.h | 1 +
drivers/s390/net/qeth_core_sys.c | 18 ++-
drivers/s390/net/qeth_l2_main.c | 176 +++++++-----------------
drivers/s390/net/qeth_l3_main.c | 112 +++++----------
drivers/s390/net/qeth_l3_sys.c | 6 +-
7 files changed, 287 insertions(+), 320 deletions(-)
--
2.16.4
^ permalink raw reply
* [PATCH net-next 03/11] s390/qeth: remove redundant netif_carrier_ok() checks
From: Julian Wiedmann @ 2018-07-19 10:43 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180719104358.79696-1-jwi@linux.ibm.com>
netif_carrier_off() does its own checking.
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 2 +-
drivers/s390/net/qeth_l2_main.c | 2 +-
drivers/s390/net/qeth_l3_main.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index f7ddd6455638..7c3b643550f6 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -653,7 +653,7 @@ static struct qeth_ipa_cmd *qeth_check_ipa_data(struct qeth_card *card,
cmd->hdr.return_code, card);
}
card->lan_online = 0;
- if (card->dev && netif_carrier_ok(card->dev))
+ if (card->dev)
netif_carrier_off(card->dev);
return NULL;
case IPA_CMD_STARTLAN:
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index 8ac243de7a9e..089bde458fd5 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -1163,7 +1163,7 @@ static int __qeth_l2_set_offline(struct ccwgroup_device *cgdev,
QETH_DBF_TEXT(SETUP, 3, "setoffl");
QETH_DBF_HEX(SETUP, 3, &card, sizeof(void *));
- if (card->dev && netif_carrier_ok(card->dev))
+ if (card->dev)
netif_carrier_off(card->dev);
recover_flag = card->state;
if ((!recovery_mode && card->info.hwtrap) || card->info.hwtrap == 2) {
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 062f62b49294..ee99af08b2c4 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -2773,7 +2773,7 @@ static int __qeth_l3_set_offline(struct ccwgroup_device *cgdev,
QETH_DBF_TEXT(SETUP, 3, "setoffl");
QETH_DBF_HEX(SETUP, 3, &card, sizeof(void *));
- if (card->dev && netif_carrier_ok(card->dev))
+ if (card->dev)
netif_carrier_off(card->dev);
recover_flag = card->state;
if ((!recovery_mode && card->info.hwtrap) || card->info.hwtrap == 2) {
--
2.16.4
^ permalink raw reply related
* Re: [PATCH iproute2-next v10] Add support for CAKE qdisc
From: Toke Høiland-Jørgensen @ 2018-07-19 10:53 UTC (permalink / raw)
To: David Ahern, netdev; +Cc: cake, Dave Taht
In-Reply-To: <ecadada4-3a27-f11c-dc8d-00998cf1d097@gmail.com>
A few comments below; will fix the rest.
>> + print_uint(PRINT_JSON, "bandwidth", NULL, bandwidth);
>> + print_string(PRINT_FP, NULL, "bandwidth %s ", sprint_rate(bandwidth, b1));
>> + } else
>> + print_string(PRINT_ANY, "bandwidth", "bandwidth %s ", "unlimited");
>> + }
>> + if (tb[TCA_CAKE_AUTORATE] &&
>> + RTA_PAYLOAD(tb[TCA_CAKE_AUTORATE]) >= sizeof(__u32)) {
>> + autorate = rta_getattr_u32(tb[TCA_CAKE_AUTORATE]);
>> + if(autorate == 1)
>> + print_string(PRINT_ANY, "autorate", "autorate_%s ", "ingress");
>> + else if(autorate)
>> + print_string(PRINT_ANY, "autorate", "(?autorate?) ", "unknown");
>
> Why the '(?' and '?)'? here and the diffserv below.
The (? ?) indicates that a value was present, but it was not understood
by tc. This has been quite useful to discover mismatch between kernel
and userspace versions of CAKE as we have been developing it.
>> + }
>> +
>> + if (tb[TCA_CAKE_NAT] &&
>> + RTA_PAYLOAD(tb[TCA_CAKE_NAT]) >= sizeof(__u32)) {
>> + nat = rta_getattr_u32(tb[TCA_CAKE_NAT]);
>> + }
>> +
>> + if(nat)
>> + print_string(PRINT_FP, NULL, "nat ", NULL);
>> + print_bool(PRINT_JSON, "nat", NULL, nat);
>
> why is the fp print under the if check but the json one is not? you
> have this in a number of places. Why not be consistent in the output?
Because JSON can actually express booleans properly, and thus we can be
explicit about its value instead of just omitting it.
-Toke
^ permalink raw reply
* [PATCH iproute2/next 0/2] set/match the tos/ttl fields of TC based IP tunnels
From: Or Gerlitz @ 2018-07-19 11:02 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Or Gerlitz
Hi Dave,
This series comes to address the case to set (encap) and match (decap)
also the tos and ttl fields of TC based IP tunnels.
Example command lines in the change log of each patch.
The kernel bits are under review [1], sending this out in parallel.
Or.
[1] https://patchwork.ozlabs.org/cover/945216/
Or Gerlitz (2):
tc/act_tunnel_key: Enable setup of tos and ttl
tc/flower: Add match on encapsulating tos/ttl
include/uapi/linux/pkt_cls.h | 5 +++
include/uapi/linux/tc_act/tc_tunnel_key.h | 2 +
man/man8/tc-flower.8 | 14 +++++++-
man/man8/tc-tunnel_key.8 | 8 ++++
tc/f_flower.c | 27 +++++++++++++++
tc/m_tunnel_key.c | 53 +++++++++++++++++++++++++++++
6 files changed, 108 insertions(+), 1 deletions(-)
^ permalink raw reply
* [PATCH iproute2/next2/next 2/2] tc/flower: Add match on encapsulating tos/ttl
From: Or Gerlitz @ 2018-07-19 11:02 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Or Gerlitz
In-Reply-To: <1531998135-13395-1-git-send-email-ogerlitz@mellanox.com>
Add matching on tos/ttl of the IP tunnel headers.
For example, here's decap rule that matches on the tunnel tos:
tc filter add dev vxlan_sys_4789 protocol ip parent ffff: prio 10 flower \
enc_src_ip 192.168.10.2 enc_dst_ip 192.168.10.1 enc_key_id 100 enc_dst_port 4789 enc_tos 0x30 \
src_mac e4:11:22:33:44:70 dst_mac e4:11:22:33:44:50 \
action tunnel_key unset \
action mirred egress redirect dev eth0_0
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/uapi/linux/pkt_cls.h | 5 +++++
man/man8/tc-flower.8 | 14 +++++++++++++-
tc/f_flower.c | 27 +++++++++++++++++++++++++++
3 files changed, 45 insertions(+), 1 deletions(-)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index c4262d9..b451225 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -473,6 +473,11 @@ enum {
TCA_FLOWER_KEY_CVLAN_PRIO, /* u8 */
TCA_FLOWER_KEY_CVLAN_ETH_TYPE, /* be16 */
+ TCA_FLOWER_KEY_ENC_IP_TOS, /* u8 */
+ TCA_FLOWER_KEY_ENC_IP_TOS_MASK, /* u8 */
+ TCA_FLOWER_KEY_ENC_IP_TTL, /* u8 */
+ TCA_FLOWER_KEY_ENC_IP_TTL_MASK, /* u8 */
+
__TCA_FLOWER_MAX,
};
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index bfa66d8..305d7ef 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -76,6 +76,10 @@ flower \- flow based traffic control filter
.IR ipv4_address " | " ipv6_address " } | "
.B enc_dst_port
.IR port_number " | "
+.B enc_tos
+.IR TOS " | "
+.B enc_ttl
+.IR TTL " | "
.BR ip_flags
.IR IP_FLAGS
.SH DESCRIPTION
@@ -275,6 +279,10 @@ bits is assumed.
.BI enc_src_ip " PREFIX"
.TQ
.BI enc_dst_port " NUMBER"
+.TQ
+.BI enc_tos " NUMBER"
+.TQ
+.BI enc_ttl " NUMBER"
Match on IP tunnel metadata. Key id
.I NUMBER
is a 32 bit tunnel key id (e.g. VNI for VXLAN tunnel).
@@ -283,7 +291,11 @@ must be a valid IPv4 or IPv6 address optionally followed by a slash and the
prefix length. If the prefix is missing, \fBtc\fR assumes a full-length
host match. Dst port
.I NUMBER
-is a 16 bit UDP dst port.
+is a 16 bit UDP dst port. Tos
+.I NUMBER
+is an 8 bit tos (dscp+ecn) value, ttl
+.I NUMBER
+is an 8 bit time-to-live value.
.TP
.BI ip_flags " IP_FLAGS"
.I IP_FLAGS
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 40b4026..a4cf06a 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -77,6 +77,8 @@ static void explain(void)
" enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
" enc_key_id [ KEY-ID ] |\n"
+ " enc_tos MASKED-IP_TOS |\n"
+ " enc_ttl MASKED-IP_TTL |\n"
" ip_flags IP-FLAGS | \n"
" enc_dst_port [ port_number ] }\n"
" FILTERID := X:Y:Z\n"
@@ -1019,6 +1021,26 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
fprintf(stderr, "Illegal \"enc_dst_port\"\n");
return -1;
}
+ } else if (matches(*argv, "enc_tos") == 0) {
+ NEXT_ARG();
+ ret = flower_parse_ip_tos_ttl(*argv,
+ TCA_FLOWER_KEY_ENC_IP_TOS,
+ TCA_FLOWER_KEY_ENC_IP_TOS_MASK,
+ n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"enc_tos\"\n");
+ return -1;
+ }
+ } else if (matches(*argv, "enc_ttl") == 0) {
+ NEXT_ARG();
+ ret = flower_parse_ip_tos_ttl(*argv,
+ TCA_FLOWER_KEY_ENC_IP_TTL,
+ TCA_FLOWER_KEY_ENC_IP_TTL_MASK,
+ n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"enc_ttl\"\n");
+ return -1;
+ }
} else if (matches(*argv, "action") == 0) {
NEXT_ARG();
ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
@@ -1542,6 +1564,11 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
flower_print_port("enc_dst_port", tb[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
+ flower_print_ip_attr("enc_tos", tb[TCA_FLOWER_KEY_ENC_IP_TOS],
+ tb[TCA_FLOWER_KEY_ENC_IP_TOS_MASK]);
+ flower_print_ip_attr("enc_ttl", tb[TCA_FLOWER_KEY_ENC_IP_TTL],
+ tb[TCA_FLOWER_KEY_ENC_IP_TTL_MASK]);
+
flower_print_matching_flags("ip_flags", FLOWER_IP_FLAGS,
tb[TCA_FLOWER_KEY_FLAGS],
tb[TCA_FLOWER_KEY_FLAGS_MASK]);
--
1.7.1
^ permalink raw reply related
* [PATCH iproute2/next 1/2] tc/act_tunnel_key: Enable setup of tos and ttl
From: Or Gerlitz @ 2018-07-19 11:02 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, Or Gerlitz
In-Reply-To: <1531998135-13395-1-git-send-email-ogerlitz@mellanox.com>
Allow to set tos and ttl for the tunnel.
For example, here's encap rule that sets tos to the tunnel:
tc filter add dev eth0_0 protocol ip parent ffff: prio 10 flower \
src_mac e4:11:22:33:44:50 dst_mac e4:11:22:33:44:70 \
action tunnel_key set src_ip 192.168.10.1 dst_ip 192.168.10.2 id 100 dst_port 4789 tos 0x30 \
action mirred egress redirect dev vxlan_sys_4789
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/uapi/linux/tc_act/tc_tunnel_key.h | 2 +
man/man8/tc-tunnel_key.8 | 8 ++++
tc/m_tunnel_key.c | 53 +++++++++++++++++++++++++++++
3 files changed, 63 insertions(+), 0 deletions(-)
diff --git a/include/uapi/linux/tc_act/tc_tunnel_key.h b/include/uapi/linux/tc_act/tc_tunnel_key.h
index e284fec..be384d6 100644
--- a/include/uapi/linux/tc_act/tc_tunnel_key.h
+++ b/include/uapi/linux/tc_act/tc_tunnel_key.h
@@ -39,6 +39,8 @@ enum {
TCA_TUNNEL_KEY_ENC_OPTS, /* Nested TCA_TUNNEL_KEY_ENC_OPTS_
* attributes
*/
+ TCA_TUNNEL_KEY_ENC_TOS, /* u8 */
+ TCA_TUNNEL_KEY_ENC_TTL, /* u8 */
__TCA_TUNNEL_KEY_MAX,
};
diff --git a/man/man8/tc-tunnel_key.8 b/man/man8/tc-tunnel_key.8
index 7d4b30e..1e09362 100644
--- a/man/man8/tc-tunnel_key.8
+++ b/man/man8/tc-tunnel_key.8
@@ -16,6 +16,8 @@ tunnel_key - Tunnel metadata manipulation
.IR ADDRESS
.BI id " KEY_ID"
.BI dst_port " UDP_PORT"
+.BI tos " TOS"
+.BI ttl " TTL"
.RB "[ " csum " | " nocsum " ]"
.SH DESCRIPTION
@@ -89,6 +91,12 @@ is specified in the form CLASS:TYPE:DATA, where CLASS is represented as a
variable length hexadecimal value. Additionally multiple options may be
listed using a comma delimiter.
.TP
+.B tos
+Outer header TOS
+.TP
+.B ttl
+Outer header TTL
+.TP
.RB [ no ] csum
Controlls outer UDP checksum. When set to
.B csum
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index 5a0e3fc..e9e71e4 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -190,6 +190,22 @@ static int tunnel_key_parse_geneve_opts(char *str, struct nlmsghdr *n)
return 0;
}
+static int tunnel_key_parse_tos_ttl(char *str, int type, struct nlmsghdr *n)
+{
+ int ret;
+ __u8 val;
+
+ ret = get_u8(&val, str, 10);
+ if (ret)
+ ret = get_u8(&val, str, 16);
+ if (ret)
+ return -1;
+
+ addattr8(n, MAX_MSG, type, val);
+
+ return 0;
+}
+
static int parse_tunnel_key(struct action_util *a, int *argc_p, char ***argv_p,
int tca_id, struct nlmsghdr *n)
{
@@ -273,6 +289,22 @@ static int parse_tunnel_key(struct action_util *a, int *argc_p, char ***argv_p,
fprintf(stderr, "Illegal \"geneve_opts\"\n");
return -1;
}
+ } else if (matches(*argv, "tos") == 0) {
+ NEXT_ARG();
+ ret = tunnel_key_parse_tos_ttl(*argv,
+ TCA_TUNNEL_KEY_ENC_TOS, n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"tos\"\n");
+ return -1;
+ }
+ } else if (matches(*argv, "ttl") == 0) {
+ NEXT_ARG();
+ ret = tunnel_key_parse_tos_ttl(*argv,
+ TCA_TUNNEL_KEY_ENC_TTL, n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"ttl\"\n");
+ return -1;
+ }
} else if (matches(*argv, "csum") == 0) {
csum = 1;
} else if (matches(*argv, "nocsum") == 0) {
@@ -435,6 +467,23 @@ static void tunnel_key_print_key_opt(const char *name, struct rtattr *attr)
tb[TCA_TUNNEL_KEY_ENC_OPTS_GENEVE]);
}
+static void tunnel_key_print_tos_ttl(FILE *f, char *name,
+ struct rtattr *attr)
+{
+ if (!attr)
+ return;
+
+ if (matches(name, "tos") == 0 && rta_getattr_u8(attr) != 0) {
+ print_string(PRINT_FP, NULL, "%s", _SL_);
+ print_uint(PRINT_ANY, "tos", "\ttos 0x%x",
+ rta_getattr_u8(attr));
+ } else if (matches(name, "ttl") == 0 && rta_getattr_u8(attr) != 0) {
+ print_string(PRINT_FP, NULL, "%s", _SL_);
+ print_uint(PRINT_ANY, "ttl", "\tttl %u",
+ rta_getattr_u8(attr));
+ }
+}
+
static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
{
struct rtattr *tb[TCA_TUNNEL_KEY_MAX + 1];
@@ -476,6 +525,10 @@ static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
tb[TCA_TUNNEL_KEY_ENC_OPTS]);
tunnel_key_print_flag(f, "nocsum", "csum",
tb[TCA_TUNNEL_KEY_NO_CSUM]);
+ tunnel_key_print_tos_ttl(f, "tos",
+ tb[TCA_TUNNEL_KEY_ENC_TOS]);
+ tunnel_key_print_tos_ttl(f, "ttl",
+ tb[TCA_TUNNEL_KEY_ENC_TTL]);
break;
}
print_action_control(f, " ", parm->action, "");
--
1.7.1
^ permalink raw reply related
* [net-next v4 0/3] net/tls: Minor code cleanup patches
From: Vakul Garg @ 2018-07-19 16:23 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
This patch series improves tls_sw.c code by:
1) Using correct socket callback for flagging data availability.
2) Removing redundant variable assignments and wakeup callbacks.
3) Removing redundant dynamic array allocation.
The patches do not fix any functional bug. Hence "Fixes:" tag has not
been used. From patch series v3, this series v4 contains two patches
less. They will be submitted separately.
Vakul Garg (3):
net/tls: Use socket data_ready callback on record availability
net/tls: Remove redundant variable assignments and wakeup
net/tls: Remove redundant array allocation.
net/tls/tls_sw.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
--
2.13.6
^ permalink raw reply
* [net-next v5 1/3] net/tls: Use socket data_ready callback on record availability
From: Vakul Garg @ 2018-07-19 16:23 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719162344.26968-1-vakul.garg@nxp.com>
On receipt of a complete tls record, use socket's saved data_ready
callback instead of state_change callback.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
net/tls/tls_sw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 7d194c0cd6cf..a58661c624ec 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1023,7 +1023,7 @@ static void tls_queue(struct strparser *strp, struct sk_buff *skb)
ctx->recv_pkt = skb;
strp_pause(strp);
- strp->sk->sk_state_change(strp->sk);
+ ctx->saved_data_ready(strp->sk);
}
static void tls_data_ready(struct sock *sk)
--
2.13.6
^ permalink raw reply related
* [net-next v5 2/3] net/tls: Remove redundant variable assignments and wakeup
From: Vakul Garg @ 2018-07-19 16:23 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719162344.26968-1-vakul.garg@nxp.com>
In function decrypt_skb_update(), the assignment to tls receive context
variable 'decrypted' is redundant as the same is being done in function
tls_sw_recvmsg() after calling decrypt_skb_update(). Also calling callback
function to wakeup processes sleeping on socket data availability is
useless as decrypt_skb_update() is invoked from user processes only. This
patch cleans these up.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
Changes from v4->v5: Fixed compilation issue.
net/tls/tls_sw.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index a58661c624ec..e15ace0ebd79 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -679,8 +679,6 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
rxm->offset += tls_ctx->rx.prepend_size;
rxm->full_len -= tls_ctx->rx.overhead_size;
tls_advance_record_sn(sk, &tls_ctx->rx);
- ctx->decrypted = true;
- ctx->saved_data_ready(sk);
return err;
}
--
2.13.6
^ permalink raw reply related
* [net-next v5 3/3] net/tls: Remove redundant array allocation.
From: Vakul Garg @ 2018-07-19 16:23 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719162344.26968-1-vakul.garg@nxp.com>
In function decrypt_skb(), array allocation in case when sgout is NULL
is unnecessary. Instead, local variable sgin_arr[] can be used.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
net/tls/tls_sw.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index e15ace0ebd79..1aa2d46713d7 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -704,7 +704,6 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
if (!sgout) {
nsg = skb_cow_data(skb, 0, &unused) + 1;
- sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
sgout = sgin;
}
@@ -725,9 +724,6 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
rxm->full_len - tls_ctx->rx.overhead_size,
skb, sk->sk_allocation);
- if (sgin != &sgin_arr[0])
- kfree(sgin);
-
return ret;
}
--
2.13.6
^ permalink raw reply related
* [net-next v5 1/3] net/tls: Use socket data_ready callback on record availability
From: Vakul Garg @ 2018-07-19 16:26 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719162613.27184-1-vakul.garg@nxp.com>
On receipt of a complete tls record, use socket's saved data_ready
callback instead of state_change callback.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
net/tls/tls_sw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 7d194c0cd6cf..a58661c624ec 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1023,7 +1023,7 @@ static void tls_queue(struct strparser *strp, struct sk_buff *skb)
ctx->recv_pkt = skb;
strp_pause(strp);
- strp->sk->sk_state_change(strp->sk);
+ ctx->saved_data_ready(strp->sk);
}
static void tls_data_ready(struct sock *sk)
--
2.13.6
^ permalink raw reply related
* [net-next v5 2/3] net/tls: Remove redundant variable assignments and wakeup
From: Vakul Garg @ 2018-07-19 16:26 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719162613.27184-1-vakul.garg@nxp.com>
In function decrypt_skb_update(), the assignment to tls receive context
variable 'decrypted' is redundant as the same is being done in function
tls_sw_recvmsg() after calling decrypt_skb_update(). Also calling callback
function to wakeup processes sleeping on socket data availability is
useless as decrypt_skb_update() is invoked from user processes only. This
patch cleans these up.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
Changes from v4->v5: Fixed compilation issue.
net/tls/tls_sw.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index a58661c624ec..e15ace0ebd79 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -679,8 +679,6 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
rxm->offset += tls_ctx->rx.prepend_size;
rxm->full_len -= tls_ctx->rx.overhead_size;
tls_advance_record_sn(sk, &tls_ctx->rx);
- ctx->decrypted = true;
- ctx->saved_data_ready(sk);
return err;
}
--
2.13.6
^ permalink raw reply related
* [net-next v5 0/3] net/tls: Minor code cleanup patches
From: Vakul Garg @ 2018-07-19 16:26 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
This patch series improves tls_sw.c code by:
1) Using correct socket callback for flagging data availability.
2) Removing redundant variable assignments and wakeup callbacks.
3) Removing redundant dynamic array allocation.
The patches do not fix any functional bug. Hence "Fixes:" tag has not
been used. From patch series v3, this series v4 contains two patches
less. They will be submitted separately.
Vakul Garg (3):
net/tls: Use socket data_ready callback on record availability
net/tls: Remove redundant variable assignments and wakeup
net/tls: Remove redundant array allocation.
net/tls/tls_sw.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
--
2.13.6
^ permalink raw reply
* [net-next v5 3/3] net/tls: Remove redundant array allocation.
From: Vakul Garg @ 2018-07-19 16:26 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
In-Reply-To: <20180719162613.27184-1-vakul.garg@nxp.com>
In function decrypt_skb(), array allocation in case when sgout is NULL
is unnecessary. Instead, local variable sgin_arr[] can be used.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
net/tls/tls_sw.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index e15ace0ebd79..1aa2d46713d7 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -704,7 +704,6 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
if (!sgout) {
nsg = skb_cow_data(skb, 0, &unused) + 1;
- sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
sgout = sgin;
}
@@ -725,9 +724,6 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
rxm->full_len - tls_ctx->rx.overhead_size,
skb, sk->sk_allocation);
- if (sgin != &sgin_arr[0])
- kfree(sgin);
-
return ret;
}
--
2.13.6
^ permalink raw reply related
* general protection fault in p9_client_prepare_req
From: syzbot @ 2018-07-19 12:09 UTC (permalink / raw)
To: davem, ericvh, linux-kernel, lucho, netdev, rminnich,
syzkaller-bugs, v9fs-developer
Hello,
syzbot found the following crash on:
HEAD commit: 1c34981993da Add linux-next specific files for 20180719
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=160ec658400000
kernel config: https://syzkaller.appspot.com/x/.config?x=7002497517b09aec
dashboard link: https://syzkaller.appspot.com/bug?extid=77a28a63a0ece0fbba97
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=17317a58400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1135e1c2400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+77a28a63a0ece0fbba97@syzkaller.appspotmail.com
RBP: 0000000000000000 R08: 0000000020000840 R09: 0000000000003831
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc432a9cd0
R13: 0030656c69662f2e R14: 64663d736e617274 R15: 0000000000000006
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
CPU: 1 PID: 4386 Comm: syz-executor227 Not tainted
4.18.0-rc5-next-20180719+ #11
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:p9_client_prepare_req.part.8+0x392/0xa00 net/9p/client.c:645
Code: ff 0f 87 6a 02 00 00 e8 bc 39 0c fa 0f be 85 f4 fe ff ff 4c 89 f2 48
c1 ea 03 89 85 f4 fe ff ff 48 b8 00 00 00 00 00 fc ff df <80> 3c 02 00 0f
85 89 05 00 00 48 b8 00 00 00 00 00 fc ff df 4c 8b
RSP: 0018:ffff8801bd8c7250 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000600040
RDX: 000000000000000a RSI: ffffffff87706d04 RDI: ffff8801b9efa378
RBP: ffff8801bd8c7380 R08: ffff8801b9f24080 R09: ffffed0037b18de4
R10: ffffed0037b18de4 R11: 0000000000000003 R12: ffff8801b9efa340
R13: 0000000000002000 R14: 0000000000000050 R15: ffff8801b9efa37c
FS: 0000000001ba1880(0000) GS:ffff8801daf00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000200 CR3: 00000001b994e000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
p9_client_prepare_req net/9p/client.c:675 [inline]
p9_client_rpc+0x242/0x1330 net/9p/client.c:675
p9_client_version net/9p/client.c:890 [inline]
p9_client_create+0xca4/0x1537 net/9p/client.c:974
v9fs_session_init+0x21a/0x1a80 fs/9p/v9fs.c:400
v9fs_mount+0x7c/0x900 fs/9p/vfs_super.c:135
legacy_get_tree+0x131/0x460 fs/fs_context.c:674
vfs_get_tree+0x1cb/0x5c0 fs/super.c:1743
do_new_mount fs/namespace.c:2603 [inline]
do_mount+0x6f2/0x1e20 fs/namespace.c:2927
ksys_mount+0x12d/0x140 fs/namespace.c:3143
__do_sys_mount fs/namespace.c:3157 [inline]
__se_sys_mount fs/namespace.c:3154 [inline]
__x64_sys_mount+0xbe/0x150 fs/namespace.c:3154
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x440719
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 5b 14 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc432a9cc8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: ffffffffffffffff RCX: 0000000000440719
RDX: 0000000020000900 RSI: 0000000020000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: 0000000020000840 R09: 0000000000003831
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc432a9cd0
R13: 0030656c69662f2e R14: 64663d736e617274 R15: 0000000000000006
Modules linked in:
Dumping ftrace buffer:
(ftrace buffer empty)
---[ end trace bae3d5802518770f ]---
RIP: 0010:p9_client_prepare_req.part.8+0x392/0xa00 net/9p/client.c:645
Code: ff 0f 87 6a 02 00 00 e8 bc 39 0c fa 0f be 85 f4 fe ff ff 4c 89 f2 48
c1 ea 03 89 85 f4 fe ff ff 48 b8 00 00 00 00 00 fc ff df <80> 3c 02 00 0f
85 89 05 00 00 48 b8 00 00 00 00 00 fc ff df 4c 8b
RSP: 0018:ffff8801bd8c7250 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000600040
RDX: 000000000000000a RSI: ffffffff87706d04 RDI: ffff8801b9efa378
RBP: ffff8801bd8c7380 R08: ffff8801b9f24080 R09: ffffed0037b18de4
R10: ffffed0037b18de4 R11: 0000000000000003 R12: ffff8801b9efa340
R13: 0000000000002000 R14: 0000000000000050 R15: ffff8801b9efa37c
FS: 0000000001ba1880(0000) GS:ffff8801daf00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000200 CR3: 00000001b994e000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH v2 1/2] tools/bpftool: ignore build products
From: Taeung Song @ 2018-07-19 12:09 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, linux-kernel
In-Reply-To: <20180719024515.6659-1-treeze.taeung@gmail.com>
I'll resend v3 because it is better to add "bpftool*.8"
to tools/bpf/bpftool/.gitignore.
Untracked files:
(use "git add <file>..." to include in what will be committed)
tools/bpf/bpftool/Documentation/bpftool-cgroup.8
tools/bpf/bpftool/Documentation/bpftool-map.8
tools/bpf/bpftool/Documentation/bpftool-perf.8
tools/bpf/bpftool/Documentation/bpftool-prog.8
tools/bpf/bpftool/Documentation/bpftool.8
Thanks,
Taeung
On 07/19/2018 11:45 AM, Taeung Song wrote:
> For untracked things of tools/bpf, add this.
>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
> tools/bpf/.gitignore | 5 +++++
> 1 file changed, 5 insertions(+)
> create mode 100644 tools/bpf/.gitignore
>
> diff --git a/tools/bpf/.gitignore b/tools/bpf/.gitignore
> new file mode 100644
> index 000000000000..dfe2bd5a4b95
> --- /dev/null
> +++ b/tools/bpf/.gitignore
> @@ -0,0 +1,5 @@
> +FEATURE-DUMP.bpf
> +bpf_asm
> +bpf_dbg
> +bpf_exp.yacc.*
> +bpf_jit_disasm
>
^ permalink raw reply
* [PATCH v3 1/2] tools/bpftool: ignore build products
From: Taeung Song @ 2018-07-19 12:10 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, linux-kernel
For untracked things of tools/bpf, add this.
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
tools/bpf/.gitignore | 5 +++++
tools/bpf/bpftool/.gitignore | 1 +
2 files changed, 6 insertions(+)
create mode 100644 tools/bpf/.gitignore
diff --git a/tools/bpf/.gitignore b/tools/bpf/.gitignore
new file mode 100644
index 000000000000..dfe2bd5a4b95
--- /dev/null
+++ b/tools/bpf/.gitignore
@@ -0,0 +1,5 @@
+FEATURE-DUMP.bpf
+bpf_asm
+bpf_dbg
+bpf_exp.yacc.*
+bpf_jit_disasm
diff --git a/tools/bpf/bpftool/.gitignore b/tools/bpf/bpftool/.gitignore
index d7e678c2d396..103cc5b5b446 100644
--- a/tools/bpf/bpftool/.gitignore
+++ b/tools/bpf/bpftool/.gitignore
@@ -1,3 +1,4 @@
*.d
bpftool
+bpftool*.8
FEATURE-DUMP.bpftool
--
2.17.1
^ permalink raw reply related
* [PATCH v3 2/2] tools/bpftool: Fix segfault case regarding 'pin' arguments
From: Taeung Song @ 2018-07-19 12:10 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann; +Cc: netdev, linux-kernel
In-Reply-To: <20180719121005.12018-1-treeze.taeung@gmail.com>
Arguments of 'pin' subcommand should be checked
at the very beginning of do_pin_any().
Otherwise segfault errors can occur when using
'map pin' or 'prog pin' commands, so fix it.
# bpftool prog pin id
Segmentation fault
Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reported-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
tools/bpf/bpftool/common.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 32f9e397a6c0..3f140eff039f 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -217,6 +217,14 @@ int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
int err;
int fd;
+ if (argc < 3) {
+ p_err("too few arguments, id ID and FILE path is required");
+ return -1;
+ } else if (argc > 3) {
+ p_err("too many arguments");
+ return -1;
+ }
+
if (!is_prefix(*argv, "id")) {
p_err("expected 'id' got %s", *argv);
return -1;
@@ -230,9 +238,6 @@ int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
}
NEXT_ARG();
- if (argc != 1)
- usage();
-
fd = get_fd_by_id(id);
if (fd < 0) {
p_err("can't get prog by id (%u): %s", id, strerror(errno));
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
From: Russell King - ARM Linux @ 2018-07-19 12:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Bartosz Golaszewski, Sekhar Nori, Kevin Hilman, Grygorii Strashko,
David S . Miller, Srinivas Kandagatla, Lukas Wunner, Rob Herring,
Florian Fainelli, Ivan Khoronzhuk, David Lechner,
Greg Kroah-Hartman, Andrew Lunn, linux-arm-kernel, linux-kernel,
linux-omap, netdev, Bartosz Golaszewski
In-Reply-To: <20180719084503.tfv6jllsukk2zv3f@mwanda>
On Thu, Jul 19, 2018 at 11:48:37AM +0300, Dan Carpenter wrote:
> On Thu, Jul 19, 2018 at 10:20:27AM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Many non-DT platforms read the MAC address from EEPROM. Usually it's
> > either done with callbacks defined in board files or from SoC-specific
> > ethernet drivers.
> >
> > In order to generalize this, try to read the MAC from nvmem in
> > eth_platform_get_mac_address() using a standard lookup name:
> > "mac-address".
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> > net/ethernet/eth.c | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> > index 39af03894598..af3b4b1b77eb 100644
> > --- a/net/ethernet/eth.c
> > +++ b/net/ethernet/eth.c
> > @@ -54,6 +54,7 @@
> > #include <linux/if_ether.h>
> > #include <linux/of_net.h>
> > #include <linux/pci.h>
> > +#include <linux/nvmem-consumer.h>
> > #include <net/dst.h>
> > #include <net/arp.h>
> > #include <net/sock.h>
> > @@ -527,8 +528,11 @@ unsigned char * __weak arch_get_platform_mac_address(void)
> >
> > int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
> > {
> > + unsigned char addrbuf[ETH_ALEN];
> > const unsigned char *addr;
> > + struct nvmem_cell *nvmem;
> > struct device_node *dp;
> > + size_t alen;
> >
> > if (dev_is_pci(dev))
> > dp = pci_device_to_OF_node(to_pci_dev(dev));
> > @@ -541,6 +545,29 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
> > if (!addr)
> > addr = arch_get_platform_mac_address();
> >
> > + if (!addr) {
> > + nvmem = nvmem_cell_get(dev, "mac-address");
> > + if (IS_ERR(nvmem) && PTR_ERR(nvmem) == -EPROBE_DEFER)
> > + /* We may have a lookup registered for MAC address but
> > + * the corresponding nvmem provider hasn't been
> > + * registered yet.
> > + */
> > + return -EPROBE_DEFER;
> > +
> > + if (!IS_ERR(nvmem)) {
> > + addr = nvmem_cell_read(nvmem, &alen);
> > + if (!IS_ERR(addr)) {
> ^^^^
> Never do success handling. Always error handling. Otherwise the code
> is indent a lot and the error handling is far from the call.
>
> > + if (alen == ETH_ALEN)
> > + ether_addr_copy(addrbuf, addr);
> > +
> > + kfree(addr);
> > + addr = alen == ETH_ALEN ? addrbuf : NULL;
> > + }
> > +
> > + nvmem_cell_put(nvmem);
> > + }
> > + }
> > +
> > if (!addr || !is_valid_ether_addr(addr))
> ^^^^
> Instead of handling the error we dereference the error pointer here.
>
> *frowny face*
>
> > return -ENODEV;
> >
> > --
>
> Maybe this?
>
> if (!addr) {
> nvmem = nvmem_cell_get(dev, "mac-address");
> if (PTR_ERR(nvmem) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
> if (IS_ERR(nvmem))
> return -ENODEV;
> addr = nvmem_cell_read(nvmem, &alen);
> if (IS_ERR(addr))
> return PTR_ERR(addr);
The problem with doing it this way is... error handling is Hard(tm).
You missed the call to nvmem_cell_put() here.
> if (alen != ETH_ALEN) {
> kfree(addr);
and again here.
> return -ENODEV;
> }
> ether_addr_copy(addrbuf, addr);
> kfree(addr);
> addr = addrbuf;
and here.
> }
> if (!is_valid_ether_addr(addr))
> return -ENODEV;
> ether_addr_copy(mac_addr, addr);
> return 0;
Without checking the semantics, a possible solution to that could be:
if (!addr) {
nvmem = nvmem_cell_get(dev, "mac-address");
if (PTR_ERR(nvmem) == -EPROBE_DEFER)
return -EPROBE_DEFER;
if (IS_ERR(nvmem))
return -ENODEV;
addr = nvmem_cell_read(nvmem, &alen);
+ nvmem_cell_put(nvmem);
if (IS_ERR(addr))
return PTR_ERR(addr);
if (alen != ETH_ALEN) {
kfree(addr);
return -ENODEV;
}
ether_addr_copy(addrbuf, addr);
kfree(addr);
addr = addrbuf;
}
if (!is_valid_ether_addr(addr))
return -ENODEV;
ether_addr_copy(mac_addr, addr);
return 0;
A potential better solution would be to put this code in a separate
function, and then do:
if (!addr)
addr = eth_platform_get_nvmem_mac_address(dev, addrbuf);
which returns either NULL or addrbuf depending on whether it failed or
succeeded, which would probably end up with cleaner code.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up
^ permalink raw reply
* Re: [PATCH iproute2/next 1/2] tc/act_tunnel_key: Enable setup of tos and ttl
From: Roman Mashak @ 2018-07-19 11:48 UTC (permalink / raw)
To: Or Gerlitz; +Cc: David Ahern, netdev
In-Reply-To: <1531998135-13395-2-git-send-email-ogerlitz@mellanox.com>
Or Gerlitz <ogerlitz@mellanox.com> writes:
> Allow to set tos and ttl for the tunnel.
>
> For example, here's encap rule that sets tos to the tunnel:
>
> tc filter add dev eth0_0 protocol ip parent ffff: prio 10 flower \
> src_mac e4:11:22:33:44:50 dst_mac e4:11:22:33:44:70 \
> action tunnel_key set src_ip 192.168.10.1 dst_ip 192.168.10.2 id 100 dst_port 4789 tos 0x30 \
> action mirred egress redirect dev vxlan_sys_4789
>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
[...]
Or, could you also update tunnel_key actions for the new options in
$(kernel)/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
once the patches are accepted ?
^ permalink raw reply
* Re: [PATCH v2 2/3] net: add support for nvmem to eth_platform_get_mac_address()
From: Dan Carpenter @ 2018-07-19 12:37 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rob Herring, Grygorii Strashko, David Lechner, Ivan Khoronzhuk,
Kevin Hilman, Greg Kroah-Hartman, Sekhar Nori, Russell King, LKML,
Andrew Lunn, Bartosz Golaszewski, Lukas Wunner,
Srinivas Kandagatla, netdev, Florian Fainelli, Linux-OMAP,
David S . Miller, arm-soc
In-Reply-To: <CAMRc=Mdu5CLA99p48nNFZBSFHBKiERzp-m59vKDDe5STjjKZNA@mail.gmail.com>
On Thu, Jul 19, 2018 at 12:06:51PM +0200, Bartosz Golaszewski wrote:
> It looks simpler as long as you don't add all the new routines
> resulting from this approach. I've just tried to quickly implement
> this solution and it resulted in much bigger and duplicated code
> (checking the validity of mac_addr, copying it etc.). I would prefer
> the current approach and would like to read someone else's opinion on
> that.
It's not too bad. There is two extra ether_addr_copy() calls and one
extra is_valid_ether_addr(). There are two extra alen declarations and
one extra addr declaration. The functions don't share *that* much code.
regards,
dan carpenter
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index fd8faa0dfa61..8ab7289a5069 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -54,6 +54,8 @@
#include <linux/if_ether.h>
#include <linux/of_net.h>
#include <linux/pci.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/mtd/mtd.h>
#include <net/dst.h>
#include <net/arp.h>
#include <net/sock.h>
@@ -525,7 +527,7 @@ unsigned char * __weak arch_get_platform_mac_address(void)
return NULL;
}
-int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
+static int of_eth_get_mac_address(struct device *dev, u8 *mac_addr)
{
const unsigned char *addr;
struct device_node *dp;
@@ -547,4 +549,82 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
ether_addr_copy(mac_addr, addr);
return 0;
}
+
+static int nvmem_eth_get_mac_address(struct device *dev, u8 *mac_addr)
+{
+ struct nvmem_cell *nvmem;
+ unsigned char *addr;
+ size_t alen;
+ int ret = 0;
+
+ nvmem = nvmem_cell_get(dev, "mac-address");
+ if (PTR_ERR(nvmem) == -EPROBE_DEFER)
+ /* We may have a lookup registered for MAC address but the
+ * corresponding nvmem provider hasn't been registered yet.
+ */
+ return -EPROBE_DEFER;
+ if (IS_ERR(nvmem))
+ return PTR_ERR(nvmem);
+ addr = nvmem_cell_read(nvmem, &alen);
+ if (IS_ERR(addr)) {
+ ret = PTR_ERR(addr);
+ goto put_nvmem;
+ }
+ if (alen != ETH_ALEN || !is_valid_ether_addr(addr)) {
+ ret = -EINVAL;
+ goto free_addr;
+ }
+ ether_addr_copy(mac_addr, addr);
+free_addr:
+ kfree(addr);
+put_nvmem:
+ nvmem_cell_put(nvmem);
+ return ret;
+}
+
+static int mtd_eth_get_mac_address(struct device *dev, u8 *mac_addr)
+{
+ struct mtd_info *mtd;
+ u8 addrbuf[ETH_ALEN];
+ size_t alen;
+ int ret;
+
+ /* This function should go away as soon as MTD gets nvmem support. */
+ if (!IS_ENABLED(CONFIG_MTD))
+ return -ENODEV;
+
+ mtd = get_mtd_device_nm("MAC-Address");
+ if (IS_ERR(mtd))
+ return PTR_ERR(mtd);
+ ret = mtd_read(mtd, 0, ETH_ALEN, &alen, addrbuf);
+ if (ret)
+ goto put_mtd;
+ if (!is_valid_ether_addr(addrbuf)) {
+ ret = -EINVAL;
+ goto put_mtd;
+ }
+ ether_addr_copy(mac_addr, addrbuf);
+put_mtd:
+ put_mtd_device(mtd);
+ return ret;
+}
+
+int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
+{
+ int ret;
+
+ ret = of_eth_get_mac_address(dev, mac_addr);
+ if (!ret)
+ return 0;
+ ret = nvmem_eth_get_mac_address(dev, mac_addr);
+ if (ret == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ if (!ret)
+ return 0;
+ ret = mtd_eth_get_mac_address(dev, mac_addr);
+ if (!ret)
+ return 0;
+
+ return -ENODEV;
+}
EXPORT_SYMBOL(eth_platform_get_mac_address);
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox