[PULL 00/17] Net patches

[PULL 00/17] Net patches

[Jason Wang]

The following changes since commit c6f28ed5075df79fef39c500362a3f4089256c9c:

  Update version for v5.2.0-rc1 release (2020-11-10 22:29:57 +0000)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to 71182187ddae5d5b17bd48464f719798321484ed:

  hw/net/can/ctucan_core: Use stl_le_p to write to tx_buffers (2020-11-11 20:34:36 +0800)

----------------------------------------------------------------

----------------------------------------------------------------
AlexChen (1):
      net/l2tpv3: Remove redundant check in net_init_l2tpv3()

Cindy Lu (1):
      virtio-net: Set mac address to hardware if the peer is vdpa

Li Zhijian (2):
      colo-compare: fix missing compare_seq initialization
      colo-compare: check mark in mutual exclusion

Pan Nengyuan (1):
      net/filter-rewriter: destroy g_hash_table in colo_rewriter_cleanup

Peter Maydell (4):
      hw/net/can/ctucan: Don't allow guest to write off end of tx_buffer
      hw/net/can/ctucan: Avoid unused value in ctucan_send_ready_buffers()
      hw/net/can/ctucan_core: Handle big-endian hosts
      hw/net/can/ctucan_core: Use stl_le_p to write to tx_buffers

Prasad J Pandit (1):
      net: remove an assert call in eth_get_gso_type

Rao, Lei (3):
      Optimize seq_sorter function for colo-compare
      Reduce the time of checkpoint for COLO
      Fix the qemu crash when guest shutdown in COLO mode

Zhang Chen (4):
      net/colo-compare.c: Fix compare_timeout format issue
      net/colo-compare.c: Change the timer clock type
      net/colo-compare.c: Add secondary old packet detection
      net/colo-compare.c: Increase default queued packet scan frequency

 hw/net/can/ctucan_core.c | 23 ++++++-------------
 hw/net/can/ctucan_core.h |  3 +--
 hw/net/virtio-net.c      |  6 +++++
 migration/ram.c          | 14 +++++++++++-
 net/colo-compare.c       | 58 +++++++++++++++++++++++++-----------------------
 net/colo.c               |  5 +----
 net/eth.c                |  6 ++---
 net/filter-rewriter.c    |  2 ++
 net/l2tpv3.c             |  9 +++-----
 softmmu/vl.c             |  1 +
 10 files changed, 67 insertions(+), 60 deletions(-)

[PULL 01/17] virtio-net: Set mac address to hardware if the peer is vdpa

[Jason Wang]

From: Cindy Lu <lulu@redhat.com>

If the peer's type is vdpa, we need to set the mac address to hardware
in virtio_net_device_realize,

Signed-off-by: Cindy Lu <lulu@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 277289d..9179013 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3395,6 +3395,12 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     nc = qemu_get_queue(n->nic);
     nc->rxfilter_notify_enabled = 1;
 
+   if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) {
+        struct virtio_net_config netcfg = {};
+        memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN);
+        vhost_net_set_config(get_vhost_net(nc->peer),
+            (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_MASTER);
+    }
     QTAILQ_INIT(&n->rsc_chains);
     n->qdev = dev;
 
-- 
2.7.4

[PULL 02/17] net/filter-rewriter: destroy g_hash_table in colo_rewriter_cleanup

[Jason Wang]

From: Pan Nengyuan <pannengyuan@huawei.com>

s->connection_track_table forgot to destroy in colo_rewriter_cleanup. Fix it.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/filter-rewriter.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index dc3c27a..e063a81 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -381,6 +381,8 @@ static void colo_rewriter_cleanup(NetFilterState *nf)
         filter_rewriter_flush(nf);
         g_free(s->incoming_queue);
     }
+
+    g_hash_table_destroy(s->connection_track_table);
 }
 
 static void colo_rewriter_setup(NetFilterState *nf, Error **errp)
-- 
2.7.4

[PULL 03/17] Optimize seq_sorter function for colo-compare

[Jason Wang]

From: "Rao, Lei" <lei.rao@intel.com>

The seq of tcp has been filled in fill_pkt_tcp_info, it
can be used directly here.

Signed-off-by: Lei Rao <lei.rao@intel.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 3a45d64..a35c10f 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -194,13 +194,10 @@ static void colo_compare_inconsistency_notify(CompareState *s)
     }
 }
 
+/* Use restricted to colo_insert_packet() */
 static gint seq_sorter(Packet *a, Packet *b, gpointer data)
 {
-    struct tcp_hdr *atcp, *btcp;
-
-    atcp = (struct tcp_hdr *)(a->transport_header);
-    btcp = (struct tcp_hdr *)(b->transport_header);
-    return ntohl(atcp->th_seq) - ntohl(btcp->th_seq);
+    return a->tcp_seq - b->tcp_seq;
 }
 
 static void fill_pkt_tcp_info(void *data, uint32_t *max_ack)
-- 
2.7.4

[PULL 04/17] Reduce the time of checkpoint for COLO

[Jason Wang]

From: "Rao, Lei" <lei.rao@intel.com>

we should set ram_bulk_stage to false after ram_state_init,
otherwise the bitmap will be unused in migration_bitmap_find_dirty.
all pages in ram cache will be flushed to the ram of secondary guest
for each checkpoint.

Signed-off-by: Lei Rao <lei.rao@intel.com>
Signed-off-by: Derek Su <dereksu@qnap.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 migration/ram.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 2da2b62..add5396 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3011,6 +3011,18 @@ static void decompress_data_with_multi_threads(QEMUFile *f,
     qemu_mutex_unlock(&decomp_done_lock);
 }
 
+ /*
+  * we must set ram_bulk_stage to false, otherwise in
+  * migation_bitmap_find_dirty the bitmap will be unused and
+  * all the pages in ram cache wil be flushed to the ram of
+  * secondary VM.
+  */
+static void colo_init_ram_state(void)
+{
+    ram_state_init(&ram_state);
+    ram_state->ram_bulk_stage = false;
+}
+
 /*
  * colo cache: this is for secondary VM, we cache the whole
  * memory of the secondary VM, it is need to hold the global lock
@@ -3054,7 +3066,7 @@ int colo_init_ram_cache(void)
         }
     }
 
-    ram_state_init(&ram_state);
+    colo_init_ram_state();
     return 0;
 }
 
-- 
2.7.4

[PULL 05/17] Fix the qemu crash when guest shutdown in COLO mode

[Jason Wang]

From: "Rao, Lei" <lei.rao@intel.com>

In COLO mode, if the startup parameters of QEMU include "no-shutdown",
QEMU will crash when the guest shutdown. The root cause is when the
guest shutdown, the state of VM will switch COLO to SHUTDOWN. When do
checkpoint again, the state will be changed to COLO. But the state
switch is undefined in runstate_transitions_def, we should add it.
This patch fixes the following:
qemu-system-x86_64: invalid runstate transition: 'shutdown' -> 'colo'
Aborted

Signed-off-by: Lei Rao <lei.rao@intel.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 softmmu/vl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index a711644..e32fd48 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -632,6 +632,7 @@ static const RunStateTransition runstate_transitions_def[] = {
     { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED },
     { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE },
     { RUN_STATE_SHUTDOWN, RUN_STATE_PRELAUNCH },
+    { RUN_STATE_SHUTDOWN, RUN_STATE_COLO },
 
     { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED },
     { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
-- 
2.7.4

[PULL 06/17] colo-compare: fix missing compare_seq initialization

[Jason Wang]

From: Li Zhijian <lizhijian@cn.fujitsu.com>

Fixes: f449c9e549c ("colo: compare the packet based on the tcp sequence
number")

Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/colo.c b/net/colo.c
index a6c66d8..ef00609 100644
--- a/net/colo.c
+++ b/net/colo.c
@@ -133,14 +133,11 @@ void reverse_connection_key(ConnectionKey *key)
 
 Connection *connection_new(ConnectionKey *key)
 {
-    Connection *conn = g_slice_new(Connection);
+    Connection *conn = g_slice_new0(Connection);
 
     conn->ip_proto = key->ip_proto;
     conn->processing = false;
-    conn->offset = 0;
     conn->tcp_state = TCPS_CLOSED;
-    conn->pack = 0;
-    conn->sack = 0;
     g_queue_init(&conn->primary_list);
     g_queue_init(&conn->secondary_list);
 
-- 
2.7.4

[PULL 07/17] colo-compare: check mark in mutual exclusion

[Jason Wang]

From: Li Zhijian <lizhijian@cn.fujitsu.com>

Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index a35c10f..8d476bb 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -477,13 +477,11 @@ sec:
             colo_release_primary_pkt(s, ppkt);
             g_queue_push_head(&conn->secondary_list, spkt);
             goto pri;
-        }
-        if (mark == COLO_COMPARE_FREE_SECONDARY) {
+        } else if (mark == COLO_COMPARE_FREE_SECONDARY) {
             conn->compare_seq = spkt->seq_end;
             packet_destroy(spkt, NULL);
             goto sec;
-        }
-        if (mark == (COLO_COMPARE_FREE_PRIMARY | COLO_COMPARE_FREE_SECONDARY)) {
+        } else if (mark == (COLO_COMPARE_FREE_PRIMARY | COLO_COMPARE_FREE_SECONDARY)) {
             conn->compare_seq = ppkt->seq_end;
             colo_release_primary_pkt(s, ppkt);
             packet_destroy(spkt, NULL);
-- 
2.7.4

[PULL 08/17] net/colo-compare.c: Fix compare_timeout format issue

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

This parameter need compare with the return of qemu_clock_get_ms(),
it is uint64_t. So we need fix this issue here.

Fixes: 9cc43c94b31 ("net/colo-compare.c: Expose "compare_timeout" to users")

Reported-by: Derek Su <dereksu@qnap.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 8d476bb..76b83a9 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -120,7 +120,7 @@ struct CompareState {
     SendCo out_sendco;
     SendCo notify_sendco;
     bool vnet_hdr;
-    uint32_t compare_timeout;
+    uint64_t compare_timeout;
     uint32_t expired_scan_cycle;
 
     /*
@@ -1076,9 +1076,9 @@ static void compare_get_timeout(Object *obj, Visitor *v,
                                 Error **errp)
 {
     CompareState *s = COLO_COMPARE(obj);
-    uint32_t value = s->compare_timeout;
+    uint64_t value = s->compare_timeout;
 
-    visit_type_uint32(v, name, &value, errp);
+    visit_type_uint64(v, name, &value, errp);
 }
 
 static void compare_set_timeout(Object *obj, Visitor *v,
@@ -1141,9 +1141,9 @@ static void set_max_queue_size(Object *obj, Visitor *v,
                                Error **errp)
 {
     Error *local_err = NULL;
-    uint32_t value;
+    uint64_t value;
 
-    visit_type_uint32(v, name, &value, &local_err);
+    visit_type_uint64(v, name, &value, &local_err);
     if (local_err) {
         goto out;
     }
@@ -1391,7 +1391,7 @@ static void colo_compare_init(Object *obj)
     object_property_add_str(obj, "notify_dev",
                             compare_get_notify_dev, compare_set_notify_dev);
 
-    object_property_add(obj, "compare_timeout", "uint32",
+    object_property_add(obj, "compare_timeout", "uint64",
                         compare_get_timeout,
                         compare_set_timeout, NULL, NULL);
 
-- 
2.7.4

[PULL 09/17] net/colo-compare.c: Change the timer clock type

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

The virtual clock only runs during the emulation. It stops
when the virtual machine is stopped.
The host clock should be used for device models that emulate accurate
real time sources. It will continue to run when the virtual machine
is suspended. COLO need to know the host time here.

Fixes: dd321ecfc2e ("colo-compare: Use IOThread to Check old packet
regularly and Process packets of the primary")

Reported-by: Derek Su <dereksu@qnap.com>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 76b83a9..1263203 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -900,7 +900,7 @@ static void check_old_packet_regular(void *opaque)
 
     /* if have old packet we will notify checkpoint */
     colo_old_packet_check(s);
-    timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+    timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_HOST) +
               s->expired_scan_cycle);
 }
 
@@ -934,10 +934,10 @@ static void colo_compare_timer_init(CompareState *s)
 {
     AioContext *ctx = iothread_get_aio_context(s->iothread);
 
-    s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_VIRTUAL,
+    s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_HOST,
                                 SCALE_MS, check_old_packet_regular,
                                 s);
-    timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+    timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_HOST) +
               s->expired_scan_cycle);
 }
 
-- 
2.7.4

[PULL 10/17] net/colo-compare.c: Add secondary old packet detection

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

Detect queued secondary packet to sync VM state in time.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 1263203..0c87fd9 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -636,19 +636,26 @@ void colo_compare_unregister_notifier(Notifier *notify)
 static int colo_old_packet_check_one_conn(Connection *conn,
                                           CompareState *s)
 {
-    GList *result = NULL;
-
-    result = g_queue_find_custom(&conn->primary_list,
-                                 &s->compare_timeout,
-                                 (GCompareFunc)colo_old_packet_check_one);
+    if (!g_queue_is_empty(&conn->primary_list)) {
+        if (g_queue_find_custom(&conn->primary_list,
+                                &s->compare_timeout,
+                                (GCompareFunc)colo_old_packet_check_one))
+            goto out;
+    }
 
-    if (result) {
-        /* Do checkpoint will flush old packet */
-        colo_compare_inconsistency_notify(s);
-        return 0;
+    if (!g_queue_is_empty(&conn->secondary_list)) {
+        if (g_queue_find_custom(&conn->secondary_list,
+                                &s->compare_timeout,
+                                (GCompareFunc)colo_old_packet_check_one))
+            goto out;
     }
 
     return 1;
+
+out:
+    /* Do checkpoint will flush old packet */
+    colo_compare_inconsistency_notify(s);
+    return 0;
 }
 
 /*
-- 
2.7.4

[PULL 11/17] net/colo-compare.c: Increase default queued packet scan frequency

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

In my test, use this default parameter looks better.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 0c87fd9..337025b 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -52,7 +52,7 @@ static NotifierList colo_compare_notifiers =
 #define COLO_COMPARE_FREE_PRIMARY     0x01
 #define COLO_COMPARE_FREE_SECONDARY   0x02
 
-#define REGULAR_PACKET_CHECK_MS 3000
+#define REGULAR_PACKET_CHECK_MS 1000
 #define DEFAULT_TIME_OUT_MS 3000
 
 /* #define DEBUG_COLO_PACKETS */
-- 
2.7.4

[PULL 12/17] net: remove an assert call in eth_get_gso_type

[Jason Wang]

From: Prasad J Pandit <pjp@fedoraproject.org>

eth_get_gso_type() routine returns segmentation offload type based on
L3 protocol type. It calls g_assert_not_reached if L3 protocol is
unknown, making the following return statement unreachable. Remove the
g_assert call, it maybe triggered by a guest user.

Reported-by: Gaoning Pan <pgn@zju.edu.cn>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/eth.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/eth.c b/net/eth.c
index 0c1d413..1e0821c 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -16,6 +16,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 #include "net/eth.h"
 #include "net/checksum.h"
 #include "net/tap.h"
@@ -71,9 +72,8 @@ eth_get_gso_type(uint16_t l3_proto, uint8_t *l3_hdr, uint8_t l4proto)
             return VIRTIO_NET_HDR_GSO_TCPV6 | ecn_state;
         }
     }
-
-    /* Unsupported offload */
-    g_assert_not_reached();
+    qemu_log_mask(LOG_UNIMP, "%s: probably not GSO frame, "
+        "unknown L3 protocol: 0x%04"PRIx16"\n", __func__, l3_proto);
 
     return VIRTIO_NET_HDR_GSO_NONE | ecn_state;
 }
-- 
2.7.4

[PULL 13/17] net/l2tpv3: Remove redundant check in net_init_l2tpv3()

[Jason Wang]

From: AlexChen <alex.chen@huawei.com>

The result has been checked to be NULL before, it cannot be NULL here,
so the check is redundant. Remove it.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: AlexChen <alex.chen@huawei.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/l2tpv3.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 55fea17..e4d4218 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -655,9 +655,8 @@ int net_init_l2tpv3(const Netdev *netdev,
         error_setg(errp, "could not bind socket err=%i", errno);
         goto outerr;
     }
-    if (result) {
-        freeaddrinfo(result);
-    }
+
+    freeaddrinfo(result);
 
     memset(&hints, 0, sizeof(hints));
 
@@ -686,9 +685,7 @@ int net_init_l2tpv3(const Netdev *netdev,
     memcpy(s->dgram_dst, result->ai_addr, result->ai_addrlen);
     s->dst_size = result->ai_addrlen;
 
-    if (result) {
-        freeaddrinfo(result);
-    }
+    freeaddrinfo(result);
 
     if (l2tpv3->has_counter && l2tpv3->counter) {
         s->has_counter = true;
-- 
2.7.4

[PULL 14/17] hw/net/can/ctucan: Don't allow guest to write off end of tx_buffer

[Jason Wang]

From: Peter Maydell <peter.maydell@linaro.org>

The ctucan device has 4 CAN bus cores, each of which has a set of 20
32-bit registers for writing the transmitted data. The registers are
however not contiguous; each core's buffers is 0x100 bytes after
the last.

We got the checks on the address wrong in the ctucan_mem_write()
function:
 * the first "is addr in range at all" check allowed
   addr == CTUCAN_CORE_MEM_SIZE, which is actually the first
   byte off the end of the range
 * the decode of addresses into core-number plus offset in the
   tx buffer for that core failed to check that the offset was
   in range, so the guest could write off the end of the
   tx_buffer[] array

NB: currently the values of CTUCAN_CORE_MEM_SIZE, CTUCAN_CORE_TXBUF_NUM,
etc, make "buff_num >= CTUCAN_CORE_TXBUF_NUM" impossible, but we
retain this as a runtime check rather than an assertion to permit
those values to be changed in future (in hardware they are
configurable synthesis parameters).

Fix the top level check, and check the offset is within the buffer.

Fixes: Coverity CID 1432874
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Tested-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/can/ctucan_core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/net/can/ctucan_core.c b/hw/net/can/ctucan_core.c
index d20835c..8486f42 100644
--- a/hw/net/can/ctucan_core.c
+++ b/hw/net/can/ctucan_core.c
@@ -303,7 +303,7 @@ void ctucan_mem_write(CtuCanCoreState *s, hwaddr addr, uint64_t val,
     DPRINTF("write 0x%02llx addr 0x%02x\n",
             (unsigned long long)val, (unsigned int)addr);
 
-    if (addr > CTUCAN_CORE_MEM_SIZE) {
+    if (addr >= CTUCAN_CORE_MEM_SIZE) {
         return;
     }
 
@@ -312,7 +312,9 @@ void ctucan_mem_write(CtuCanCoreState *s, hwaddr addr, uint64_t val,
         addr -= CTU_CAN_FD_TXTB1_DATA_1;
         buff_num = addr / CTUCAN_CORE_TXBUFF_SPAN;
         addr %= CTUCAN_CORE_TXBUFF_SPAN;
-        if (buff_num < CTUCAN_CORE_TXBUF_NUM) {
+        addr &= ~3;
+        if ((buff_num < CTUCAN_CORE_TXBUF_NUM) &&
+            (addr < sizeof(s->tx_buffer[buff_num].data))) {
             uint32_t *bufp = (uint32_t *)(s->tx_buffer[buff_num].data + addr);
             *bufp = cpu_to_le32(val);
         }
-- 
2.7.4

[PULL 15/17] hw/net/can/ctucan: Avoid unused value in ctucan_send_ready_buffers()

[Jason Wang]

From: Peter Maydell <peter.maydell@linaro.org>

Coverity points out that in ctucan_send_ready_buffers() we
set buff_st_mask = 0xf << (i * 4) inside the loop, but then
we never use it before overwriting it later.

The only thing we use the mask for is as part of the code that is
inserting the new buff_st field into tx_status.  That is more
comprehensibly written using deposit32(), so do that and drop the
mask variable entirely.

We also update the buff_st local variable at multiple points
during this function, but nothing can ever see these
intermediate values, so just drop those, write the final
TXT_TOK as a fixed constant value, and collapse the only
remaining set/use of buff_st down into an extract32().

Fixes: Coverity CID 1432869
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Acked-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Tested-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/can/ctucan_core.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/hw/net/can/ctucan_core.c b/hw/net/can/ctucan_core.c
index 8486f42..f49c762 100644
--- a/hw/net/can/ctucan_core.c
+++ b/hw/net/can/ctucan_core.c
@@ -240,8 +240,6 @@ static void ctucan_send_ready_buffers(CtuCanCoreState *s)
     uint8_t *pf;
     int buff2tx_idx;
     uint32_t tx_prio_max;
-    unsigned int buff_st;
-    uint32_t buff_st_mask;
 
     if (!s->mode_settings.s.ena) {
         return;
@@ -256,10 +254,7 @@ static void ctucan_send_ready_buffers(CtuCanCoreState *s)
         for (i = 0; i < CTUCAN_CORE_TXBUF_NUM; i++) {
             uint32_t prio;
 
-            buff_st_mask = 0xf << (i * 4);
-            buff_st = (s->tx_status.u32 >> (i * 4)) & 0xf;
-
-            if (buff_st != TXT_RDY) {
+            if (extract32(s->tx_status.u32, i * 4, 4) != TXT_RDY) {
                 continue;
             }
             prio = (s->tx_priority.u32 >> (i * 4)) & 0x7;
@@ -271,10 +266,7 @@ static void ctucan_send_ready_buffers(CtuCanCoreState *s)
         if (buff2tx_idx == -1) {
             break;
         }
-        buff_st_mask = 0xf << (buff2tx_idx * 4);
-        buff_st = (s->tx_status.u32 >> (buff2tx_idx * 4)) & 0xf;
         int_stat.u32 = 0;
-        buff_st = TXT_RDY;
         pf = s->tx_buffer[buff2tx_idx].data;
         ctucan_buff2frame(pf, &frame);
         s->status.s.idle = 0;
@@ -283,12 +275,11 @@ static void ctucan_send_ready_buffers(CtuCanCoreState *s)
         s->status.s.idle = 1;
         s->status.s.txs = 0;
         s->tx_fr_ctr.s.tx_fr_ctr_val++;
-        buff_st = TXT_TOK;
         int_stat.s.txi = 1;
         int_stat.s.txbhci = 1;
         s->int_stat.u32 |= int_stat.u32 & ~s->int_mask.u32;
-        s->tx_status.u32 = (s->tx_status.u32 & ~buff_st_mask) |
-                        (buff_st << (buff2tx_idx * 4));
+        s->tx_status.u32 = deposit32(s->tx_status.u32,
+                                     buff2tx_idx * 4, 4, TXT_TOK);
     } while (1);
 }
 
-- 
2.7.4

[PULL 16/17] hw/net/can/ctucan_core: Handle big-endian hosts

[Jason Wang]

From: Peter Maydell <peter.maydell@linaro.org>

The ctucan driver defines types for its registers which are a union
of a uint32_t with a struct with bitfields for the individual
fields within that register. This is a bad idea, because bitfields
aren't portable. The ctu_can_fd_regs.h header works around the
most glaring of the portability issues by defining the
fields in two different orders depending on the setting of the
__LITTLE_ENDIAN_BITFIELD define. However, in ctucan_core.h this
is unconditionally set to 1, which is wrong for big-endian hosts.

Set it only if HOST_WORDS_BIGENDIAN is not set. There is no need
for a "have we defined it already" guard, because the only place
that should set it is ctucan_core.h, which has the usual
double-inclusion guard.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Acked-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Tested-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/can/ctucan_core.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/net/can/ctucan_core.h b/hw/net/can/ctucan_core.h
index f21cb1c..bbc09ae 100644
--- a/hw/net/can/ctucan_core.h
+++ b/hw/net/can/ctucan_core.h
@@ -31,8 +31,7 @@
 #include "exec/hwaddr.h"
 #include "net/can_emu.h"
 
-
-#ifndef __LITTLE_ENDIAN_BITFIELD
+#ifndef HOST_WORDS_BIGENDIAN
 #define __LITTLE_ENDIAN_BITFIELD 1
 #endif
 
-- 
2.7.4

[PULL 17/17] hw/net/can/ctucan_core: Use stl_le_p to write to tx_buffers

[Jason Wang]

From: Peter Maydell <peter.maydell@linaro.org>

Instead of casting an address within a uint8_t array to a
uint32_t*, use stl_le_p(). This handles possibly misaligned
addresses which would otherwise crash on some hosts.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Tested-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/can/ctucan_core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/net/can/ctucan_core.c b/hw/net/can/ctucan_core.c
index f49c762..d171c37 100644
--- a/hw/net/can/ctucan_core.c
+++ b/hw/net/can/ctucan_core.c
@@ -303,11 +303,9 @@ void ctucan_mem_write(CtuCanCoreState *s, hwaddr addr, uint64_t val,
         addr -= CTU_CAN_FD_TXTB1_DATA_1;
         buff_num = addr / CTUCAN_CORE_TXBUFF_SPAN;
         addr %= CTUCAN_CORE_TXBUFF_SPAN;
-        addr &= ~3;
         if ((buff_num < CTUCAN_CORE_TXBUF_NUM) &&
-            (addr < sizeof(s->tx_buffer[buff_num].data))) {
-            uint32_t *bufp = (uint32_t *)(s->tx_buffer[buff_num].data + addr);
-            *bufp = cpu_to_le32(val);
+            ((addr + size) <= sizeof(s->tx_buffer[buff_num].data))) {
+            stn_le_p(s->tx_buffer[buff_num].data + addr, size, val);
         }
     } else {
         switch (addr & ~3) {
-- 
2.7.4

Re: [PULL 00/17] Net patches

[Peter Maydell]

On Wed, 11 Nov 2020 at 13:11, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit c6f28ed5075df79fef39c500362a3f4089256c9c:
>
>   Update version for v5.2.0-rc1 release (2020-11-10 22:29:57 +0000)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 71182187ddae5d5b17bd48464f719798321484ed:
>
>   hw/net/can/ctucan_core: Use stl_le_p to write to tx_buffers (2020-11-11 20:34:36 +0800)
>
> ----------------------------------------------------------------
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM

[PULL 00/17] Net patches

[Jason Wang]

The following changes since commit 03a3a62fbd0aa5227e978eef3c67d3978aec9e5f:

  Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2023-09-07 10:29:06 -0400)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to 049cfda145e96b2605cdf9739f1bcf9ebf3a83e1:

  ebpf: Updated eBPF program and skeleton. (2023-09-08 14:33:46 +0800)

----------------------------------------------------------------

----------------------------------------------------------------
Andrew Melnychenko (7):
      tap: Add USO support to tap device.
      virtio-net: Add USO flags to vhost support.
      ebpf: Added eBPF map update through mmap.
      ebpf: Added eBPF initialization by fds.
      virtio-net: Added property to load eBPF RSS with fds.
      qmp: Added new command to retrieve eBPF blob.
      ebpf: Updated eBPF program and skeleton.

Ilya Maximets (1):
      net: add initial support for AF_XDP network backend

Tomasz Dzieciol (7):
      igb: remove TCP ACK detection
      igb: rename E1000E_RingInfo_st
      igb: RX descriptors guest writting refactoring
      igb: RX payload guest writting refactoring
      igb: add IPv6 extended headers traffic detection
      igb: packet-split descriptors support
      e1000e: rename e1000e_ba_state and e1000e_write_hdr_to_rx_buffers

Yuri Benditovich (2):
      tap: Add check for USO features
      virtio-net: Add support for USO features

 MAINTAINERS                                     |    4 +
 ebpf/ebpf.c                                     |   70 ++
 ebpf/ebpf.h                                     |   31 +
 ebpf/ebpf_rss-stub.c                            |    6 +
 ebpf/ebpf_rss.c                                 |  150 ++-
 ebpf/ebpf_rss.h                                 |   10 +
 ebpf/meson.build                                |    2 +-
 ebpf/rss.bpf.skeleton.h                         | 1460 ++++++++++++-----------
 hmp-commands.hx                                 |    3 +
 hw/core/machine.c                               |    4 +
 hw/net/e1000e_core.c                            |   80 +-
 hw/net/igb_core.c                               |  732 ++++++++----
 hw/net/igb_regs.h                               |   20 +-
 hw/net/trace-events                             |    6 +-
 hw/net/vhost_net.c                              |    3 +
 hw/net/virtio-net.c                             |   90 +-
 hw/net/vmxnet3.c                                |    2 +
 include/hw/virtio/virtio-net.h                  |    1 +
 include/net/net.h                               |    7 +-
 meson.build                                     |   19 +-
 meson_options.txt                               |    2 +
 net/af-xdp.c                                    |  526 ++++++++
 net/clients.h                                   |    5 +
 net/meson.build                                 |    3 +
 net/net.c                                       |   19 +-
 net/tap-bsd.c                                   |    7 +-
 net/tap-linux.c                                 |   27 +-
 net/tap-linux.h                                 |    2 +
 net/tap-solaris.c                               |    7 +-
 net/tap-stub.c                                  |    7 +-
 net/tap-win32.c                                 |    2 +-
 net/tap.c                                       |   18 +-
 net/tap_int.h                                   |    4 +-
 net/vhost-vdpa.c                                |    3 +
 qapi/ebpf.json                                  |   66 +
 qapi/meson.build                                |    1 +
 qapi/net.json                                   |   58 +
 qapi/qapi-schema.json                           |    1 +
 qemu-options.hx                                 |   70 +-
 scripts/ci/org.centos/stream/8/x86_64/configure |    1 +
 scripts/meson-buildoptions.sh                   |    3 +
 tests/docker/dockerfiles/debian-amd64.docker    |    1 +
 tests/qtest/libqos/igb.c                        |    5 +
 tools/ebpf/rss.bpf.c                            |    5 +-
 44 files changed, 2518 insertions(+), 1025 deletions(-)
 create mode 100644 ebpf/ebpf.c
 create mode 100644 ebpf/ebpf.h
 create mode 100644 net/af-xdp.c
 create mode 100644 qapi/ebpf.json

Re: [PULL 00/17] Net patches

[Stefan Hajnoczi]

Hi Ilya and Jason,
There is a CI failure related to a missing Debian libxdp-dev package:
https://gitlab.com/qemu-project/qemu/-/jobs/5046139967

I think the issue is that the debian-amd64 container image that QEMU
uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
and libxdp is not available on that release:
https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all

If we need to support Debian 11 CI then either XDP could be disabled
for that distro or libxdp could be compiled from source.

I have CCed Daniel Berrangé, because I think he knows how lcitool and
QEMU's minimum distro requirements work. Maybe he can suggest a way
forward.

Thanks,
Stefan

Re: [PULL 00/17] Net patches

[Ilya Maximets]

On 9/8/23 13:19, Stefan Hajnoczi wrote:
> Hi Ilya and Jason,
> There is a CI failure related to a missing Debian libxdp-dev package:
> https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
> 
> I think the issue is that the debian-amd64 container image that QEMU
> uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
> and libxdp is not available on that release:
> https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all

Hmm.  Sorry about that.

> 
> If we need to support Debian 11 CI then either XDP could be disabled
> for that distro or libxdp could be compiled from source.

I'd suggest we just remove the attempt to install the package for now,
building libxdp from sources may be a little painful to maintain.

Can be re-added later once distributions with libxdp 1.4+ will be more
widely available, i.e. when fedora dockerfile will be updated to 39,
for example.  That should be soon-ish, right?

> 
> I have CCed Daniel Berrangé, because I think he knows how lcitool and
> QEMU's minimum distro requirements work. Maybe he can suggest a way
> forward.
> 
> Thanks,
> Stefan

Re: [PULL 00/17] Net patches

[Daniel P. Berrangé]

On Fri, Sep 08, 2023 at 01:34:54PM +0200, Ilya Maximets wrote:
> On 9/8/23 13:19, Stefan Hajnoczi wrote:
> > Hi Ilya and Jason,
> > There is a CI failure related to a missing Debian libxdp-dev package:
> > https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
> > 
> > I think the issue is that the debian-amd64 container image that QEMU
> > uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
> > and libxdp is not available on that release:
> > https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all
> 
> Hmm.  Sorry about that.
> 
> > 
> > If we need to support Debian 11 CI then either XDP could be disabled
> > for that distro or libxdp could be compiled from source.
> 
> I'd suggest we just remove the attempt to install the package for now,
> building libxdp from sources may be a little painful to maintain.
> 
> Can be re-added later once distributions with libxdp 1.4+ will be more
> widely available, i.e. when fedora dockerfile will be updated to 39,
> for example.  That should be soon-ish, right?

If you follow the process in docs/devel/testing.rst for adding
libxdp in libvirt-ci, then lcitool will "do the right thing"
when we move the auto-generated dockerfiles to new distro versions.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 00/17] Net patches

[Ilya Maximets]

On 9/8/23 13:49, Daniel P. Berrangé wrote:
> On Fri, Sep 08, 2023 at 01:34:54PM +0200, Ilya Maximets wrote:
>> On 9/8/23 13:19, Stefan Hajnoczi wrote:
>>> Hi Ilya and Jason,
>>> There is a CI failure related to a missing Debian libxdp-dev package:
>>> https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
>>>
>>> I think the issue is that the debian-amd64 container image that QEMU
>>> uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
>>> and libxdp is not available on that release:
>>> https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all
>>
>> Hmm.  Sorry about that.
>>
>>>
>>> If we need to support Debian 11 CI then either XDP could be disabled
>>> for that distro or libxdp could be compiled from source.
>>
>> I'd suggest we just remove the attempt to install the package for now,
>> building libxdp from sources may be a little painful to maintain.
>>
>> Can be re-added later once distributions with libxdp 1.4+ will be more
>> widely available, i.e. when fedora dockerfile will be updated to 39,
>> for example.  That should be soon-ish, right?
> 
> If you follow the process in docs/devel/testing.rst for adding
> libxdp in libvirt-ci, then lcitool will "do the right thing"
> when we move the auto-generated dockerfiles to new distro versions.

Thanks!  I'll prepare changes for libvirt-ci.

In the meantime, none of the currently tested images will have a required
version of libxdp anyway, so I'm suggesting to just drop this one dockerfile
modification from the patch.  What do you think?

Best regards, Ilya Maximets.

Re: [PULL 00/17] Net patches

[Daniel P. Berrangé]

On Fri, Sep 08, 2023 at 02:00:47PM +0200, Ilya Maximets wrote:
> On 9/8/23 13:49, Daniel P. Berrangé wrote:
> > On Fri, Sep 08, 2023 at 01:34:54PM +0200, Ilya Maximets wrote:
> >> On 9/8/23 13:19, Stefan Hajnoczi wrote:
> >>> Hi Ilya and Jason,
> >>> There is a CI failure related to a missing Debian libxdp-dev package:
> >>> https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
> >>>
> >>> I think the issue is that the debian-amd64 container image that QEMU
> >>> uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
> >>> and libxdp is not available on that release:
> >>> https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all
> >>
> >> Hmm.  Sorry about that.
> >>
> >>>
> >>> If we need to support Debian 11 CI then either XDP could be disabled
> >>> for that distro or libxdp could be compiled from source.
> >>
> >> I'd suggest we just remove the attempt to install the package for now,
> >> building libxdp from sources may be a little painful to maintain.
> >>
> >> Can be re-added later once distributions with libxdp 1.4+ will be more
> >> widely available, i.e. when fedora dockerfile will be updated to 39,
> >> for example.  That should be soon-ish, right?
> > 
> > If you follow the process in docs/devel/testing.rst for adding
> > libxdp in libvirt-ci, then lcitool will "do the right thing"
> > when we move the auto-generated dockerfiles to new distro versions.
> 
> Thanks!  I'll prepare changes for libvirt-ci.
> 
> In the meantime, none of the currently tested images will have a required
> version of libxdp anyway, so I'm suggesting to just drop this one dockerfile
> modification from the patch.  What do you think?

Sure, if none of the distros have it, then lcitool won't emit the
dockerfile changes until we update the inherited distro version.
So it is sufficient to just update libvirt-ci.git with the mappings.yml
info for libxdp, and add 'libxdp' to the tests/lcitool/projects/qemu.yml
file in qemu.git. It will then 'just work' when someone updates the
distro versions later.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 00/17] Net patches

[Ilya Maximets]

On 9/8/23 14:15, Daniel P. Berrangé wrote:
> On Fri, Sep 08, 2023 at 02:00:47PM +0200, Ilya Maximets wrote:
>> On 9/8/23 13:49, Daniel P. Berrangé wrote:
>>> On Fri, Sep 08, 2023 at 01:34:54PM +0200, Ilya Maximets wrote:
>>>> On 9/8/23 13:19, Stefan Hajnoczi wrote:
>>>>> Hi Ilya and Jason,
>>>>> There is a CI failure related to a missing Debian libxdp-dev package:
>>>>> https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
>>>>>
>>>>> I think the issue is that the debian-amd64 container image that QEMU
>>>>> uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
>>>>> and libxdp is not available on that release:
>>>>> https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all
>>>>
>>>> Hmm.  Sorry about that.
>>>>
>>>>>
>>>>> If we need to support Debian 11 CI then either XDP could be disabled
>>>>> for that distro or libxdp could be compiled from source.
>>>>
>>>> I'd suggest we just remove the attempt to install the package for now,
>>>> building libxdp from sources may be a little painful to maintain.
>>>>
>>>> Can be re-added later once distributions with libxdp 1.4+ will be more
>>>> widely available, i.e. when fedora dockerfile will be updated to 39,
>>>> for example.  That should be soon-ish, right?
>>>
>>> If you follow the process in docs/devel/testing.rst for adding
>>> libxdp in libvirt-ci, then lcitool will "do the right thing"
>>> when we move the auto-generated dockerfiles to new distro versions.
>>
>> Thanks!  I'll prepare changes for libvirt-ci.
>>
>> In the meantime, none of the currently tested images will have a required
>> version of libxdp anyway, so I'm suggesting to just drop this one dockerfile
>> modification from the patch.  What do you think?
> 
> Sure, if none of the distros have it, then lcitool won't emit the
> dockerfile changes until we update the inherited distro version.
> So it is sufficient to just update libvirt-ci.git with the mappings.yml
> info for libxdp, and add 'libxdp' to the tests/lcitool/projects/qemu.yml
> file in qemu.git. It will then 'just work' when someone updates the
> distro versions later.

I posted an MR for libvirt-ci adding libxdp:
  https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/429

Please, take a look.

The docs say that CI will try to build containers with the MR changes,
but I don't think anything except sanity checks is actually tested on MR.
Sorry if I missed something, never used GitLab pipelines before.

Note that with this update we will be installing older version of libxdp
in many containers, even though they will not be used by QEMU, unless
they are newer than 1.4.0.

tests/lcitool/projects/qemu.yml in qemu.git cannot be updated without
updating a submodule after the MR merge.

Best regards, Ilya Maximets.

Re: [PULL 00/17] Net patches

[Daniel P. Berrangé]

On Fri, Sep 08, 2023 at 04:06:35PM +0200, Ilya Maximets wrote:
> On 9/8/23 14:15, Daniel P. Berrangé wrote:
> > On Fri, Sep 08, 2023 at 02:00:47PM +0200, Ilya Maximets wrote:
> >> On 9/8/23 13:49, Daniel P. Berrangé wrote:
> >>> On Fri, Sep 08, 2023 at 01:34:54PM +0200, Ilya Maximets wrote:
> >>>> On 9/8/23 13:19, Stefan Hajnoczi wrote:
> >>>>> Hi Ilya and Jason,
> >>>>> There is a CI failure related to a missing Debian libxdp-dev package:
> >>>>> https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
> >>>>>
> >>>>> I think the issue is that the debian-amd64 container image that QEMU
> >>>>> uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
> >>>>> and libxdp is not available on that release:
> >>>>> https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all
> >>>>
> >>>> Hmm.  Sorry about that.
> >>>>
> >>>>>
> >>>>> If we need to support Debian 11 CI then either XDP could be disabled
> >>>>> for that distro or libxdp could be compiled from source.
> >>>>
> >>>> I'd suggest we just remove the attempt to install the package for now,
> >>>> building libxdp from sources may be a little painful to maintain.
> >>>>
> >>>> Can be re-added later once distributions with libxdp 1.4+ will be more
> >>>> widely available, i.e. when fedora dockerfile will be updated to 39,
> >>>> for example.  That should be soon-ish, right?
> >>>
> >>> If you follow the process in docs/devel/testing.rst for adding
> >>> libxdp in libvirt-ci, then lcitool will "do the right thing"
> >>> when we move the auto-generated dockerfiles to new distro versions.
> >>
> >> Thanks!  I'll prepare changes for libvirt-ci.
> >>
> >> In the meantime, none of the currently tested images will have a required
> >> version of libxdp anyway, so I'm suggesting to just drop this one dockerfile
> >> modification from the patch.  What do you think?
> > 
> > Sure, if none of the distros have it, then lcitool won't emit the
> > dockerfile changes until we update the inherited distro version.
> > So it is sufficient to just update libvirt-ci.git with the mappings.yml
> > info for libxdp, and add 'libxdp' to the tests/lcitool/projects/qemu.yml
> > file in qemu.git. It will then 'just work' when someone updates the
> > distro versions later.
> 
> I posted an MR for libvirt-ci adding libxdp:
>   https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/429
> 
> Please, take a look.
> 
> The docs say that CI will try to build containers with the MR changes,
> but I don't think anything except sanity checks is actually tested on MR.
> Sorry if I missed something, never used GitLab pipelines before.

No, that's our fault - we've broken the CI and your change alerted
me to that fact :-)

> Note that with this update we will be installing older version of libxdp
> in many containers, even though they will not be used by QEMU, unless
> they are newer than 1.4.0.

No problem, as it means QEMU CI will demonstrate the the meson.build
change is ignoring the outdatd libxdp.

> tests/lcitool/projects/qemu.yml in qemu.git cannot be updated without
> updating a submodule after the MR merge.

Yep.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 00/17] Net patches

[Ilya Maximets]

On 9/8/23 16:15, Daniel P. Berrangé wrote:
> On Fri, Sep 08, 2023 at 04:06:35PM +0200, Ilya Maximets wrote:
>> On 9/8/23 14:15, Daniel P. Berrangé wrote:
>>> On Fri, Sep 08, 2023 at 02:00:47PM +0200, Ilya Maximets wrote:
>>>> On 9/8/23 13:49, Daniel P. Berrangé wrote:
>>>>> On Fri, Sep 08, 2023 at 01:34:54PM +0200, Ilya Maximets wrote:
>>>>>> On 9/8/23 13:19, Stefan Hajnoczi wrote:
>>>>>>> Hi Ilya and Jason,
>>>>>>> There is a CI failure related to a missing Debian libxdp-dev package:
>>>>>>> https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
>>>>>>>
>>>>>>> I think the issue is that the debian-amd64 container image that QEMU
>>>>>>> uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
>>>>>>> and libxdp is not available on that release:
>>>>>>> https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all
>>>>>>
>>>>>> Hmm.  Sorry about that.
>>>>>>
>>>>>>>
>>>>>>> If we need to support Debian 11 CI then either XDP could be disabled
>>>>>>> for that distro or libxdp could be compiled from source.
>>>>>>
>>>>>> I'd suggest we just remove the attempt to install the package for now,
>>>>>> building libxdp from sources may be a little painful to maintain.
>>>>>>
>>>>>> Can be re-added later once distributions with libxdp 1.4+ will be more
>>>>>> widely available, i.e. when fedora dockerfile will be updated to 39,
>>>>>> for example.  That should be soon-ish, right?
>>>>>
>>>>> If you follow the process in docs/devel/testing.rst for adding
>>>>> libxdp in libvirt-ci, then lcitool will "do the right thing"
>>>>> when we move the auto-generated dockerfiles to new distro versions.
>>>>
>>>> Thanks!  I'll prepare changes for libvirt-ci.
>>>>
>>>> In the meantime, none of the currently tested images will have a required
>>>> version of libxdp anyway, so I'm suggesting to just drop this one dockerfile
>>>> modification from the patch.  What do you think?
>>>
>>> Sure, if none of the distros have it, then lcitool won't emit the
>>> dockerfile changes until we update the inherited distro version.
>>> So it is sufficient to just update libvirt-ci.git with the mappings.yml
>>> info for libxdp, and add 'libxdp' to the tests/lcitool/projects/qemu.yml
>>> file in qemu.git. It will then 'just work' when someone updates the
>>> distro versions later.
>>
>> I posted an MR for libvirt-ci adding libxdp:
>>   https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/429
>>
>> Please, take a look.
>>
>> The docs say that CI will try to build containers with the MR changes,
>> but I don't think anything except sanity checks is actually tested on MR.
>> Sorry if I missed something, never used GitLab pipelines before.
> 
> No, that's our fault - we've broken the CI and your change alerted
> me to that fact :-)
> 
>> Note that with this update we will be installing older version of libxdp
>> in many containers, even though they will not be used by QEMU, unless
>> they are newer than 1.4.0.
> 
> No problem, as it means QEMU CI will demonstrate the the meson.build
> change is ignoring the outdatd libxdp.
> 
>> tests/lcitool/projects/qemu.yml in qemu.git cannot be updated without
>> updating a submodule after the MR merge.
> 
> Yep.

Since all the required changes went into libvirt-ci project, I posted an
updated patch set named:

  '[PATCH v4 0/2] net: add initial support for AF_XDP network backend'

Please, take a look.

This should fix the CI issues, though I'm not sure how to run QEMU gitlab
pipelines myself, so I didn't actually test all the images.

Sent as a patch set because the libvirt-ci submodule bump brings in a few
unrelated changes.  So, I split that into a separate patch.

Best regards, Ilya Maximets.

Re: [PULL 00/17] Net patches

[Daniel P. Berrangé]

On Wed, Sep 13, 2023 at 08:46:42PM +0200, Ilya Maximets wrote:
> On 9/8/23 16:15, Daniel P. Berrangé wrote:
> > On Fri, Sep 08, 2023 at 04:06:35PM +0200, Ilya Maximets wrote:
> >> On 9/8/23 14:15, Daniel P. Berrangé wrote:
> >>> On Fri, Sep 08, 2023 at 02:00:47PM +0200, Ilya Maximets wrote:
> >>>> On 9/8/23 13:49, Daniel P. Berrangé wrote:
> >>>>> On Fri, Sep 08, 2023 at 01:34:54PM +0200, Ilya Maximets wrote:
> >>>>>> On 9/8/23 13:19, Stefan Hajnoczi wrote:
> >>>>>>> Hi Ilya and Jason,
> >>>>>>> There is a CI failure related to a missing Debian libxdp-dev package:
> >>>>>>> https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
> >>>>>>>
> >>>>>>> I think the issue is that the debian-amd64 container image that QEMU
> >>>>>>> uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
> >>>>>>> and libxdp is not available on that release:
> >>>>>>> https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all
> >>>>>>
> >>>>>> Hmm.  Sorry about that.
> >>>>>>
> >>>>>>>
> >>>>>>> If we need to support Debian 11 CI then either XDP could be disabled
> >>>>>>> for that distro or libxdp could be compiled from source.
> >>>>>>
> >>>>>> I'd suggest we just remove the attempt to install the package for now,
> >>>>>> building libxdp from sources may be a little painful to maintain.
> >>>>>>
> >>>>>> Can be re-added later once distributions with libxdp 1.4+ will be more
> >>>>>> widely available, i.e. when fedora dockerfile will be updated to 39,
> >>>>>> for example.  That should be soon-ish, right?
> >>>>>
> >>>>> If you follow the process in docs/devel/testing.rst for adding
> >>>>> libxdp in libvirt-ci, then lcitool will "do the right thing"
> >>>>> when we move the auto-generated dockerfiles to new distro versions.
> >>>>
> >>>> Thanks!  I'll prepare changes for libvirt-ci.
> >>>>
> >>>> In the meantime, none of the currently tested images will have a required
> >>>> version of libxdp anyway, so I'm suggesting to just drop this one dockerfile
> >>>> modification from the patch.  What do you think?
> >>>
> >>> Sure, if none of the distros have it, then lcitool won't emit the
> >>> dockerfile changes until we update the inherited distro version.
> >>> So it is sufficient to just update libvirt-ci.git with the mappings.yml
> >>> info for libxdp, and add 'libxdp' to the tests/lcitool/projects/qemu.yml
> >>> file in qemu.git. It will then 'just work' when someone updates the
> >>> distro versions later.
> >>
> >> I posted an MR for libvirt-ci adding libxdp:
> >>   https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/429
> >>
> >> Please, take a look.
> >>
> >> The docs say that CI will try to build containers with the MR changes,
> >> but I don't think anything except sanity checks is actually tested on MR.
> >> Sorry if I missed something, never used GitLab pipelines before.
> > 
> > No, that's our fault - we've broken the CI and your change alerted
> > me to that fact :-)
> > 
> >> Note that with this update we will be installing older version of libxdp
> >> in many containers, even though they will not be used by QEMU, unless
> >> they are newer than 1.4.0.
> > 
> > No problem, as it means QEMU CI will demonstrate the the meson.build
> > change is ignoring the outdatd libxdp.
> > 
> >> tests/lcitool/projects/qemu.yml in qemu.git cannot be updated without
> >> updating a submodule after the MR merge.
> > 
> > Yep.
> 
> Since all the required changes went into libvirt-ci project, I posted an
> updated patch set named:
> 
>   '[PATCH v4 0/2] net: add initial support for AF_XDP network backend'
> 
> Please, take a look.
> 
> This should fix the CI issues, though I'm not sure how to run QEMU gitlab
> pipelines myself, so I didn't actually test all the images.

  git push gitlab  -o ci.variable=QEMU_CI=2

will create pipeline and immediately run all jobs.

Replace 'gitlab' with the name of the git remote pointing to your
gitlab fork of QEMU.

Using QEMU_CI=1 will create pipeline, but let you manually start
individual jobs from the web UI.

For further details see docs/devel/ci-jobs.rst.inc


> 
> Sent as a patch set because the libvirt-ci submodule bump brings in a few
> unrelated changes.  So, I split that into a separate patch.

Yep, that's perfect thanks.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 00/17] Net patches

[Ilya Maximets]

On 9/14/23 10:13, Daniel P. Berrangé wrote:
> On Wed, Sep 13, 2023 at 08:46:42PM +0200, Ilya Maximets wrote:
>> On 9/8/23 16:15, Daniel P. Berrangé wrote:
>>> On Fri, Sep 08, 2023 at 04:06:35PM +0200, Ilya Maximets wrote:
>>>> On 9/8/23 14:15, Daniel P. Berrangé wrote:
>>>>> On Fri, Sep 08, 2023 at 02:00:47PM +0200, Ilya Maximets wrote:
>>>>>> On 9/8/23 13:49, Daniel P. Berrangé wrote:
>>>>>>> On Fri, Sep 08, 2023 at 01:34:54PM +0200, Ilya Maximets wrote:
>>>>>>>> On 9/8/23 13:19, Stefan Hajnoczi wrote:
>>>>>>>>> Hi Ilya and Jason,
>>>>>>>>> There is a CI failure related to a missing Debian libxdp-dev package:
>>>>>>>>> https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
>>>>>>>>>
>>>>>>>>> I think the issue is that the debian-amd64 container image that QEMU
>>>>>>>>> uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
>>>>>>>>> and libxdp is not available on that release:
>>>>>>>>> https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all
>>>>>>>>
>>>>>>>> Hmm.  Sorry about that.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> If we need to support Debian 11 CI then either XDP could be disabled
>>>>>>>>> for that distro or libxdp could be compiled from source.
>>>>>>>>
>>>>>>>> I'd suggest we just remove the attempt to install the package for now,
>>>>>>>> building libxdp from sources may be a little painful to maintain.
>>>>>>>>
>>>>>>>> Can be re-added later once distributions with libxdp 1.4+ will be more
>>>>>>>> widely available, i.e. when fedora dockerfile will be updated to 39,
>>>>>>>> for example.  That should be soon-ish, right?
>>>>>>>
>>>>>>> If you follow the process in docs/devel/testing.rst for adding
>>>>>>> libxdp in libvirt-ci, then lcitool will "do the right thing"
>>>>>>> when we move the auto-generated dockerfiles to new distro versions.
>>>>>>
>>>>>> Thanks!  I'll prepare changes for libvirt-ci.
>>>>>>
>>>>>> In the meantime, none of the currently tested images will have a required
>>>>>> version of libxdp anyway, so I'm suggesting to just drop this one dockerfile
>>>>>> modification from the patch.  What do you think?
>>>>>
>>>>> Sure, if none of the distros have it, then lcitool won't emit the
>>>>> dockerfile changes until we update the inherited distro version.
>>>>> So it is sufficient to just update libvirt-ci.git with the mappings.yml
>>>>> info for libxdp, and add 'libxdp' to the tests/lcitool/projects/qemu.yml
>>>>> file in qemu.git. It will then 'just work' when someone updates the
>>>>> distro versions later.
>>>>
>>>> I posted an MR for libvirt-ci adding libxdp:
>>>>   https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/429
>>>>
>>>> Please, take a look.
>>>>
>>>> The docs say that CI will try to build containers with the MR changes,
>>>> but I don't think anything except sanity checks is actually tested on MR.
>>>> Sorry if I missed something, never used GitLab pipelines before.
>>>
>>> No, that's our fault - we've broken the CI and your change alerted
>>> me to that fact :-)
>>>
>>>> Note that with this update we will be installing older version of libxdp
>>>> in many containers, even though they will not be used by QEMU, unless
>>>> they are newer than 1.4.0.
>>>
>>> No problem, as it means QEMU CI will demonstrate the the meson.build
>>> change is ignoring the outdatd libxdp.
>>>
>>>> tests/lcitool/projects/qemu.yml in qemu.git cannot be updated without
>>>> updating a submodule after the MR merge.
>>>
>>> Yep.
>>
>> Since all the required changes went into libvirt-ci project, I posted an
>> updated patch set named:
>>
>>   '[PATCH v4 0/2] net: add initial support for AF_XDP network backend'
>>
>> Please, take a look.
>>
>> This should fix the CI issues, though I'm not sure how to run QEMU gitlab
>> pipelines myself, so I didn't actually test all the images.
> 
>   git push gitlab  -o ci.variable=QEMU_CI=2
> 
> will create pipeline and immediately run all jobs.

Thanks!  That worked.  Though I wasn't able to test much anyway as
this thing burned through all my free compute credits less than
half way through the pipeline. :D

So, AFAIU, it's not something an occasional contributor like me can
use, unless they are spending their own money.


> 
> Replace 'gitlab' with the name of the git remote pointing to your
> gitlab fork of QEMU.
> 
> Using QEMU_CI=1 will create pipeline, but let you manually start
> individual jobs from the web UI.
> 
> For further details see docs/devel/ci-jobs.rst.inc
> 
> 
>>
>> Sent as a patch set because the libvirt-ci submodule bump brings in a few
>> unrelated changes.  So, I split that into a separate patch.
> 
> Yep, that's perfect thanks.
> 
> With regards,
> Daniel

Re: [PULL 00/17] Net patches

[Daniel P. Berrangé]

On Mon, Sep 18, 2023 at 09:36:10PM +0200, Ilya Maximets wrote:
> On 9/14/23 10:13, Daniel P. Berrangé wrote:
> > On Wed, Sep 13, 2023 at 08:46:42PM +0200, Ilya Maximets wrote:
> >> On 9/8/23 16:15, Daniel P. Berrangé wrote:
> >>> On Fri, Sep 08, 2023 at 04:06:35PM +0200, Ilya Maximets wrote:
> >>>> On 9/8/23 14:15, Daniel P. Berrangé wrote:
> >>>>> On Fri, Sep 08, 2023 at 02:00:47PM +0200, Ilya Maximets wrote:
> >>>>>> On 9/8/23 13:49, Daniel P. Berrangé wrote:
> >>>>>>> On Fri, Sep 08, 2023 at 01:34:54PM +0200, Ilya Maximets wrote:
> >>>>>>>> On 9/8/23 13:19, Stefan Hajnoczi wrote:
> >>>>>>>>> Hi Ilya and Jason,
> >>>>>>>>> There is a CI failure related to a missing Debian libxdp-dev package:
> >>>>>>>>> https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
> >>>>>>>>>
> >>>>>>>>> I think the issue is that the debian-amd64 container image that QEMU
> >>>>>>>>> uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
> >>>>>>>>> and libxdp is not available on that release:
> >>>>>>>>> https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all
> >>>>>>>>
> >>>>>>>> Hmm.  Sorry about that.
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> If we need to support Debian 11 CI then either XDP could be disabled
> >>>>>>>>> for that distro or libxdp could be compiled from source.
> >>>>>>>>
> >>>>>>>> I'd suggest we just remove the attempt to install the package for now,
> >>>>>>>> building libxdp from sources may be a little painful to maintain.
> >>>>>>>>
> >>>>>>>> Can be re-added later once distributions with libxdp 1.4+ will be more
> >>>>>>>> widely available, i.e. when fedora dockerfile will be updated to 39,
> >>>>>>>> for example.  That should be soon-ish, right?
> >>>>>>>
> >>>>>>> If you follow the process in docs/devel/testing.rst for adding
> >>>>>>> libxdp in libvirt-ci, then lcitool will "do the right thing"
> >>>>>>> when we move the auto-generated dockerfiles to new distro versions.
> >>>>>>
> >>>>>> Thanks!  I'll prepare changes for libvirt-ci.
> >>>>>>
> >>>>>> In the meantime, none of the currently tested images will have a required
> >>>>>> version of libxdp anyway, so I'm suggesting to just drop this one dockerfile
> >>>>>> modification from the patch.  What do you think?
> >>>>>
> >>>>> Sure, if none of the distros have it, then lcitool won't emit the
> >>>>> dockerfile changes until we update the inherited distro version.
> >>>>> So it is sufficient to just update libvirt-ci.git with the mappings.yml
> >>>>> info for libxdp, and add 'libxdp' to the tests/lcitool/projects/qemu.yml
> >>>>> file in qemu.git. It will then 'just work' when someone updates the
> >>>>> distro versions later.
> >>>>
> >>>> I posted an MR for libvirt-ci adding libxdp:
> >>>>   https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/429
> >>>>
> >>>> Please, take a look.
> >>>>
> >>>> The docs say that CI will try to build containers with the MR changes,
> >>>> but I don't think anything except sanity checks is actually tested on MR.
> >>>> Sorry if I missed something, never used GitLab pipelines before.
> >>>
> >>> No, that's our fault - we've broken the CI and your change alerted
> >>> me to that fact :-)
> >>>
> >>>> Note that with this update we will be installing older version of libxdp
> >>>> in many containers, even though they will not be used by QEMU, unless
> >>>> they are newer than 1.4.0.
> >>>
> >>> No problem, as it means QEMU CI will demonstrate the the meson.build
> >>> change is ignoring the outdatd libxdp.
> >>>
> >>>> tests/lcitool/projects/qemu.yml in qemu.git cannot be updated without
> >>>> updating a submodule after the MR merge.
> >>>
> >>> Yep.
> >>
> >> Since all the required changes went into libvirt-ci project, I posted an
> >> updated patch set named:
> >>
> >>   '[PATCH v4 0/2] net: add initial support for AF_XDP network backend'
> >>
> >> Please, take a look.
> >>
> >> This should fix the CI issues, though I'm not sure how to run QEMU gitlab
> >> pipelines myself, so I didn't actually test all the images.
> > 
> >   git push gitlab  -o ci.variable=QEMU_CI=2
> > 
> > will create pipeline and immediately run all jobs.
> 
> Thanks!  That worked.  Though I wasn't able to test much anyway as
> this thing burned through all my free compute credits less than
> half way through the pipeline. :D
> 
> So, AFAIU, it's not something an occasional contributor like me can
> use, unless they are spending their own money.

That is not the expected behaviour.

If your repo is a fork of https://gitlab.com/qemu-project/qemu it
should benefit from a *massive* x125 reduction on CI costs.

The critical thing is that it *MUST* have been created with the
'Fork' button on qemu-project/qemu. If that's not the case then
you will burn CI credits at a cost of 1 minute == 1 credit,
instead of 1 minute == 0.008 credits. Check this by going to
the top page of your repo, and looking for a box a little above
the file list, that says

    "Forked from QEMU / QEMU"

If that is not the case, then you'll have to rename your existing
repo to get it out of the way, and then use the 'Fork' button to
create a new copy that is tracked as a fork.

With most accounts getting 400 CI minutes per month, an averge
QEMU CI run should consume about 7 CI minutes.

NB, CI credits reset on the 1st of each month

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 00/17] Net patches

[Ilya Maximets]

On 9/19/23 10:40, Daniel P. Berrangé wrote:
> On Mon, Sep 18, 2023 at 09:36:10PM +0200, Ilya Maximets wrote:
>> On 9/14/23 10:13, Daniel P. Berrangé wrote:
>>> On Wed, Sep 13, 2023 at 08:46:42PM +0200, Ilya Maximets wrote:
>>>> On 9/8/23 16:15, Daniel P. Berrangé wrote:
>>>>> On Fri, Sep 08, 2023 at 04:06:35PM +0200, Ilya Maximets wrote:
>>>>>> On 9/8/23 14:15, Daniel P. Berrangé wrote:
>>>>>>> On Fri, Sep 08, 2023 at 02:00:47PM +0200, Ilya Maximets wrote:
>>>>>>>> On 9/8/23 13:49, Daniel P. Berrangé wrote:
>>>>>>>>> On Fri, Sep 08, 2023 at 01:34:54PM +0200, Ilya Maximets wrote:
>>>>>>>>>> On 9/8/23 13:19, Stefan Hajnoczi wrote:
>>>>>>>>>>> Hi Ilya and Jason,
>>>>>>>>>>> There is a CI failure related to a missing Debian libxdp-dev package:
>>>>>>>>>>> https://gitlab.com/qemu-project/qemu/-/jobs/5046139967
>>>>>>>>>>>
>>>>>>>>>>> I think the issue is that the debian-amd64 container image that QEMU
>>>>>>>>>>> uses for testing is based on Debian 11 ("bullseye" aka "oldstable")
>>>>>>>>>>> and libxdp is not available on that release:
>>>>>>>>>>> https://packages.debian.org/search?keywords=libxdp&searchon=names&suite=oldstable&section=all
>>>>>>>>>>
>>>>>>>>>> Hmm.  Sorry about that.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> If we need to support Debian 11 CI then either XDP could be disabled
>>>>>>>>>>> for that distro or libxdp could be compiled from source.
>>>>>>>>>>
>>>>>>>>>> I'd suggest we just remove the attempt to install the package for now,
>>>>>>>>>> building libxdp from sources may be a little painful to maintain.
>>>>>>>>>>
>>>>>>>>>> Can be re-added later once distributions with libxdp 1.4+ will be more
>>>>>>>>>> widely available, i.e. when fedora dockerfile will be updated to 39,
>>>>>>>>>> for example.  That should be soon-ish, right?
>>>>>>>>>
>>>>>>>>> If you follow the process in docs/devel/testing.rst for adding
>>>>>>>>> libxdp in libvirt-ci, then lcitool will "do the right thing"
>>>>>>>>> when we move the auto-generated dockerfiles to new distro versions.
>>>>>>>>
>>>>>>>> Thanks!  I'll prepare changes for libvirt-ci.
>>>>>>>>
>>>>>>>> In the meantime, none of the currently tested images will have a required
>>>>>>>> version of libxdp anyway, so I'm suggesting to just drop this one dockerfile
>>>>>>>> modification from the patch.  What do you think?
>>>>>>>
>>>>>>> Sure, if none of the distros have it, then lcitool won't emit the
>>>>>>> dockerfile changes until we update the inherited distro version.
>>>>>>> So it is sufficient to just update libvirt-ci.git with the mappings.yml
>>>>>>> info for libxdp, and add 'libxdp' to the tests/lcitool/projects/qemu.yml
>>>>>>> file in qemu.git. It will then 'just work' when someone updates the
>>>>>>> distro versions later.
>>>>>>
>>>>>> I posted an MR for libvirt-ci adding libxdp:
>>>>>>   https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/429
>>>>>>
>>>>>> Please, take a look.
>>>>>>
>>>>>> The docs say that CI will try to build containers with the MR changes,
>>>>>> but I don't think anything except sanity checks is actually tested on MR.
>>>>>> Sorry if I missed something, never used GitLab pipelines before.
>>>>>
>>>>> No, that's our fault - we've broken the CI and your change alerted
>>>>> me to that fact :-)
>>>>>
>>>>>> Note that with this update we will be installing older version of libxdp
>>>>>> in many containers, even though they will not be used by QEMU, unless
>>>>>> they are newer than 1.4.0.
>>>>>
>>>>> No problem, as it means QEMU CI will demonstrate the the meson.build
>>>>> change is ignoring the outdatd libxdp.
>>>>>
>>>>>> tests/lcitool/projects/qemu.yml in qemu.git cannot be updated without
>>>>>> updating a submodule after the MR merge.
>>>>>
>>>>> Yep.
>>>>
>>>> Since all the required changes went into libvirt-ci project, I posted an
>>>> updated patch set named:
>>>>
>>>>   '[PATCH v4 0/2] net: add initial support for AF_XDP network backend'
>>>>
>>>> Please, take a look.
>>>>
>>>> This should fix the CI issues, though I'm not sure how to run QEMU gitlab
>>>> pipelines myself, so I didn't actually test all the images.
>>>
>>>   git push gitlab  -o ci.variable=QEMU_CI=2
>>>
>>> will create pipeline and immediately run all jobs.
>>
>> Thanks!  That worked.  Though I wasn't able to test much anyway as
>> this thing burned through all my free compute credits less than
>> half way through the pipeline. :D
>>
>> So, AFAIU, it's not something an occasional contributor like me can
>> use, unless they are spending their own money.
> 
> That is not the expected behaviour.
> 
> If your repo is a fork of https://gitlab.com/qemu-project/qemu it
> should benefit from a *massive* x125 reduction on CI costs.
> 
> The critical thing is that it *MUST* have been created with the
> 'Fork' button on qemu-project/qemu.

Yeah, it might be that the problem is caused by me accidentally
forking the gitlab.com/qemu/qemu repo instead of qemu-project.

It is fairly confusing that qemu/qemu is not the main repository
of QEMU project.  It seems to be some sort of automated account
and it closely follows updates of the main repository.  It also
has a better name, and it is *not a fork* of the qemu-project.
There practically no indication that qemu/qemu is not a main
repository, except for an icon and a lower star/fork count.
It's easy to fork the wrong one.

Do you folks have control over this account?  Could you maybe add
a description that it is not the official QEMU repository and add
a link to qemu-project?  Right now qemu-project/qemu states that
it is a "QEMU main repository", but qemu/qemu doesn't say anything.


> If that's not the case then
> you will burn CI credits at a cost of 1 minute == 1 credit,
> instead of 1 minute == 0.008 credits. Check this by going to
> the top page of your repo, and looking for a box a little above
> the file list, that says
> 
>     "Forked from QEMU / QEMU"
> 
> If that is not the case, then you'll have to rename your existing
> repo to get it out of the way, and then use the 'Fork' button to
> create a new copy that is tracked as a fork.
> 
> With most accounts getting 400 CI minutes per month, an averge
> QEMU CI run should consume about 7 CI minutes.
> 
> NB, CI credits reset on the 1st of each month

I deleted my fork (there wasn't anything useful there) and re-forked
the correct one.  Will try again next month.  For now "No more CI
minutes available.  You have used 788 out of 400 of your shared Runners
pipeline minutes." :D

Best regards, Ilya Maximets.

Re: [PULL 00/17] Net patches

[Daniel P. Berrangé]

On Tue, Sep 19, 2023 at 11:39:31AM +0200, Ilya Maximets wrote:
> On 9/19/23 10:40, Daniel P. Berrangé wrote:
> > On Mon, Sep 18, 2023 at 09:36:10PM +0200, Ilya Maximets wrote:
> >> Thanks!  That worked.  Though I wasn't able to test much anyway as
> >> this thing burned through all my free compute credits less than
> >> half way through the pipeline. :D
> >>
> >> So, AFAIU, it's not something an occasional contributor like me can
> >> use, unless they are spending their own money.
> > 
> > That is not the expected behaviour.
> > 
> > If your repo is a fork of https://gitlab.com/qemu-project/qemu it
> > should benefit from a *massive* x125 reduction on CI costs.
> > 
> > The critical thing is that it *MUST* have been created with the
> > 'Fork' button on qemu-project/qemu.
> 
> Yeah, it might be that the problem is caused by me accidentally
> forking the gitlab.com/qemu/qemu repo instead of qemu-project.
> 
> It is fairly confusing that qemu/qemu is not the main repository
> of QEMU project.  It seems to be some sort of automated account
> and it closely follows updates of the main repository.  It also
> has a better name, and it is *not a fork* of the qemu-project.
> There practically no indication that qemu/qemu is not a main
> repository, except for an icon and a lower star/fork count.
> It's easy to fork the wrong one.
> 
> Do you folks have control over this account?  Could you maybe add
> a description that it is not the official QEMU repository and add
> a link to qemu-project?  Right now qemu-project/qemu states that
> it is a "QEMU main repository", but qemu/qemu doesn't say anything.

The https://gitlab.com/qemu account is a user profile who has
been squatting on that name for a while. I vaguely recall we
tried to claim it with gitlab but because it regularly pushes
code it isn't considered inactive and thus there's nothing we
can do about it :-(


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|