[ovs-dev] Inspecção Máquinas e Equipamentos de Trabalho - Lisboa
QVO LEGIS - FORMAÇÃO E CONSULTADORIA Formação certificada Inspeção Máquinas e Equipamentos de Trabalho - Dec. lei 50/2005 Formação Obrigatória DURAÇÃO 07 Horas - 1 dia HORÁRIO 9H Recepção dos participantes, início às 9H30 e fim às 17H30 Local/Data: LISBOA - 23 de SETEMBRO 2019 PORTO - 30 de SETEMBRO 2019 Nota:Ultimas inscrições PREÇO: 160 Euros (Isento de IVA) Um equipamento de trabalho é uma máquina, aparelho, ferramenta ou instalação utilizado no trabalho em todos os sectores de atividade. O Decreto-Lei 50/2005 de 25 de fevereiro estabelece que o empregador é obrigado a proceder a verificações e/ou ensaios, tendo em conta os riscos associados aos equipamentos de trabalho. A verificação periódica da conformidade dos equipamentos de trabalho consiste numa inspeção aos componentes mecânicos, elétricos e de segurança dos mesmos. Este Decreto-lei define que a verificação e ensaios devem ser realizados por pessoa competente. Destinatários Diretores e Quadros superiores Responsáveis pela manutenção dos equipamentos e das instalações, responsáveis de produção e da Qualidade. Técnicos superiores de higiene e segurança no trabalho. Responsáveis da Qualidade e Coordenadores de Segurança. Técnicos ou Téc. Superiores de Higiene e Segurança. Consultores. Responsáveis e diretores de manutenção. Todos aqueles que pretendam adquirir ou aperfeiçoar conhecimentos. Asistência técnica, frio,electricidade e mecânica. Objetivo Geral Pretende-se que os formandos sejam capazes de reconhecer o DL 50/2005 e as principais consequências decorrentes da legislação. Dotar os formandos das práticas corretas relativas á Inspecção de Máquinas e Equipamentos de Trabalho; Riscos dos Equipamentos. Conteúdo Programático Marcação CE Enquadramento legal Colocação no mercado e avaliação da conformidade DL 50/2005 de 25 de fevereiro; Características de segurança dos equipamentos; Riscos decorrentes da utilização dos equipamentos; Sistemas mecânicos de proteção; Conversão do DL50/2005 em checklist de trabalho; Estudo de caso, apresentação de um equipamento com aplicação da checklist. Metodologia Expositiva e activa com estudo de casos e análise de legislação. Formador Engº Eliseu Realinho Licenciado em Engenharia Mecânica, pós-graduado em Higiene e Segurança no Trabalho, Mestre em Riscos e Protecção Civil. Possui uma larga experiência na área da Qualidade, nomeadamente em controlo da qualidade; auditorias internas; implementação e gestão de SGQ; entre outros. Desempenhou funções de Gestor da Qualidade, Ambiente e Segurança numa empresa multinacional. Possui também uma vasta experiência como formador para a área da Qualidade e HST. Formador certificado INFORMAÇÕES E PEDIDO DE INSCRIÇÃO: (+351) 967 201 906 | (+351) 214 047 831 |forma...@qvolegis.pt Reenvie a um/a amigo/a! Obrigado! Esta newsletter, visa informar as ações de formação disponiveis para a sua valorização pessoal e profissional. A QVOLEGIS utiliza os dados dos seus clientes e potenciais clientes de acordo com a legislação do Regulamento Geral da Proteção de Dados (lei 67/98 de 26/10). Assim, se não desejar receber as nossas informações clique em Remover -ge...@qvolegis.pt, se pretender alterar os seus dados de contacto, por favor envie email para ge...@qvolegis.pt Estamos gratos pela sua atenção e vontade de continuar a receber as nossas informações. Before printing this email, please think if you really need to. Trees are each time less and less. Esta mensagem pode conter informação confidencial ou legalmente protegida para uso exclusivo do destinatário. Se não for o destinatário pretendido da mesma, não deverá assim usar, copiar, distribuir ou revelar o seu conteúdo (incluindo quaisquer anexos) a terceiros, sem a devida autorização. Se recebeu esta mensagem por engano, por favor informe o emissor por E-mail e elimine-a imediatamente. Muito Obrigado. This message may contain confidential information or privileged material, and is intended only for the individual(s) named. If you are not in the named addressee you should not disseminate, distribute or copy this e-mail (including attachments). Please notify the sender immediately by e-mail if you have received this e-mail by mistake and delete this e-mail from your system. Thank you. Remover - ge...@qvolegis.pt --- Este e-mail foi verificado em termos de vírus pelo software antivírus Avast. https://www.avast.com/antivirus ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v9 2/2] netdev-dpdk:Detailed packet drop statistics
Hi,
Addressed comments given in patch v8. Kindly review patch v9.
Changes since patch v8 :
Rebased my changes on top of Ilya's patch " Reuse vhost function for dpdk
ETH custom stats". Patch set 1/2.
Addressed comments given in patch v8.
Thanks & Regards,
Sriram.
-Original Message-
From: Sriram Vatala
Sent: 21 September 2019 08:10
To: ovs-dev@openvswitch.org; i.maxim...@samsung.com
Cc: ktray...@redhat.com; Sriram Vatala
Subject: [PATCH v9 2/2] netdev-dpdk:Detailed packet drop statistics
OVS may be unable to transmit packets for multiple reasons and today there
is a single counter to track packets dropped due to any of those reasons.
The most common reason is that a VM is unable to read packets fast enough
causing the vhostuser port transmit queue on the OVS side to become full.
This manifests as a problem with VNFs not receiving all packets. Having a
separate drop counter to track packets dropped because the transmit queue is
full will clearly indicate that the problem is on the VM side and not in
OVS. Similarly maintaining separate counters for all possible drops helps in
indicating sensible cause for packet drops.
This patch adds custom software stats counters to track packets dropped at
port level and these counters are displayed along with other stats in
"ovs-vsctl get interface statistics"
command. The detailed stats will be available for both dpdk and vhostuser
ports.
--
Changes since v8:
Addressed comments given by Ilya.
Signed-off-by: Sriram Vatala
---
Documentation/topics/dpdk/vhost-user.rst | 13 ++-
lib/netdev-dpdk.c | 81 +++
utilities/bugtool/automake.mk | 3 +-
utilities/bugtool/ovs-bugtool-get-port-stats | 25 ++
.../plugins/network-status/openvswitch.xml| 1 +
5 files changed, 105 insertions(+), 18 deletions(-) create mode 100755
utilities/bugtool/ovs-bugtool-get-port-stats
diff --git a/Documentation/topics/dpdk/vhost-user.rst
b/Documentation/topics/dpdk/vhost-user.rst
index 724aa62f6..89388a2bf 100644
--- a/Documentation/topics/dpdk/vhost-user.rst
+++ b/Documentation/topics/dpdk/vhost-user.rst
@@ -551,7 +551,18 @@ processing packets at the required rate.
The amount of Tx retries on a vhost-user or vhost-user-client interface can
be shown with::
- $ ovs-vsctl get Interface dpdkvhostclient0 statistics:tx_retries
+ $ ovs-vsctl get Interface dpdkvhostclient0
+ statistics:netdev_dpdk_tx_retries
+
+When the guest is not able to consume the packets fast enough, the
+transmit queue of the port gets filled up i.e queue runs out of free
descriptors.
+This is the most likely reason why dpdk transmit API will fail to send
+packets besides other reasons.
+
+The amount of tx failure drops on a dpdk vhost/physical interface can
+be shown with::
+
+ $ ovs-vsctl get Interface dpdkvhostclient0 \
+statistics:netdev_dpdk_tx_failure_drops
vhost-user Dequeue Zero Copy (experimental)
---
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
652b57e3b..fd8f9102e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -174,6 +174,20 @@ static const struct vhost_device_ops
virtio_net_device_ops =
.destroy_connection = destroy_connection, };
+/* Custom software stats for dpdk ports */ struct netdev_dpdk_sw_stats
+{
+/* No. of retries when unable to transmit. */
+uint64_t tx_retries;
+/* Packet drops when unable to transmit; Probably Tx queue is full. */
+uint64_t tx_failure_drops;
+/* Packet length greater than device MTU. */
+uint64_t tx_mtu_exceeded_drops;
+/* Packet drops in egress policer processing. */
+uint64_t tx_qos_drops;
+/* Packet drops in ingress policer processing. */
+uint64_t rx_qos_drops;
+};
+
enum { DPDK_RING_SIZE = 256 };
BUILD_ASSERT_DECL(IS_POW2(DPDK_RING_SIZE));
enum { DRAIN_TSC = 20ULL };
@@ -413,11 +427,10 @@ struct netdev_dpdk {
PADDED_MEMBERS(CACHE_LINE_SIZE,
struct netdev_stats stats;
-/* Custom stat for retries when unable to transmit. */
-uint64_t tx_retries;
+struct netdev_dpdk_sw_stats *sw_stats;
/* Protects stats */
rte_spinlock_t stats_lock;
-/* 4 pad bytes here. */
+/* 36 pad bytes here. */
);
PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1173,7 +1186,9 @@ common_construct(struct netdev *netdev, dpdk_port_t
port_no,
dev->rte_xstats_ids = NULL;
dev->rte_xstats_ids_size = 0;
-dev->tx_retries = (dev->type == DPDK_DEV_VHOST) ? 0 : UINT64_MAX;
+dev->sw_stats = (struct netdev_dpdk_sw_stats *)
+xzalloc(sizeof *dev->sw_stats);
+dev->sw_stats->tx_retries = (dev->type == DPDK_DEV_VHOST) ? 0 :
+ UINT64_MAX;
return 0;
}
@@ -1359,6 +1374,7 @@ common_destruct(struct netdev_dpdk *dev)
ovs_list_remove(>list_node);
free(ovsrcu_get_protected(struct ingress_policer *,
>ingress_policer));
+
[ovs-dev] [PATCH v1] odp-util: calc checksum of ip hdr for tunnel encap
From: Martin Zhang
When parse tnl_push, if IPv4 is used,
we forget to fill the ipv4 checksum fields.
In the patch:
csum has been used as a variable,
so we neeed rename it to udp_csum.
Signed-off-by: Martin Zhang
Signed-off-by: Dujie
---
lib/odp-util.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index fe59a56..fe539ec 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1482,7 +1482,7 @@ ovs_parse_tnl_push(const char *s, struct
ovs_action_push_tnl *data)
struct gre_base_hdr *greh;
struct erspan_base_hdr *ersh;
struct erspan_md2 *md2;
-uint16_t gre_proto, gre_flags, dl_type, udp_src, udp_dst, csum, sid;
+uint16_t gre_proto, gre_flags, dl_type, udp_src, udp_dst, udp_csum, sid;
ovs_be32 sip, dip;
uint32_t tnl_type = 0, header_len = 0, ip_len = 0, erspan_idx = 0;
void *l3, *l4;
@@ -1516,6 +1516,7 @@ ovs_parse_tnl_push(const char *s, struct
ovs_action_push_tnl *data)
if (eth->eth_type == htons(ETH_TYPE_IP)) {
/* IPv4 */
uint16_t ip_frag_off;
+ memset(ip, 0, sizeof(*ip));
if (!ovs_scan_len(s, ,
"ipv4(src="IP_SCAN_FMT",dst="IP_SCAN_FMT",proto=%"SCNi8
",tos=%"SCNi8",ttl=%"SCNi8",frag=0x%"SCNx16"),",
IP_SCAN_ARGS(),
@@ -1529,6 +1530,7 @@ ovs_parse_tnl_push(const char *s, struct
ovs_action_push_tnl *data)
ip->ip_frag_off = htons(ip_frag_off);
ip->ip_ihl_ver = IP_IHL_VER(5, 4);
ip_len = sizeof *ip;
+ ip->ip_csum = csum(ip, ip_len);
} else {
char sip6_s[IPV6_SCAN_LEN + 1];
char dip6_s[IPV6_SCAN_LEN + 1];
@@ -1557,13 +1559,13 @@ ovs_parse_tnl_push(const char *s, struct
ovs_action_push_tnl *data)
udp = (struct udp_header *) l4;
greh = (struct gre_base_hdr *) l4;
if (ovs_scan_len(s, ,
"udp(src=%"SCNi16",dst=%"SCNi16",csum=0x%"SCNx16"),",
- _src, _dst, )) {
+ _src, _dst, _csum)) {
uint32_t vx_flags, vni;
udp->udp_src = htons(udp_src);
udp->udp_dst = htons(udp_dst);
udp->udp_len = 0;
-udp->udp_csum = htons(csum);
+udp->udp_csum = htons(udp_csum);
if (ovs_scan_len(s, , "vxlan(flags=0x%"SCNx32",vni=0x%"SCNx32"))",
_flags, )) {
@@ -1629,6 +1631,7 @@ ovs_parse_tnl_push(const char *s, struct
ovs_action_push_tnl *data)
ovs_16aligned_be32 *options = (ovs_16aligned_be32 *) (greh + 1);
if (greh->flags & htons(GRE_CSUM)) {
+uint16_t csum;
if (!ovs_scan_len(s, , ",csum=0x%"SCNx16, )) {
return -EINVAL;
}
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch v3] conntrack: Add option to disable TCP sequence checking.
This may be needed in some special cases, such as to support some hardware
offload implementations. Note that disabling TCP sequence number
verification is not an optimization in itself, but supporting some
hardware offload implementations may offer better performance. TCP
sequence number verification is enabled by default. This option is only
available for the userspace datapath. Access to this option is presently
provided via 'dpctl' commands as the need for this option is quite node
specific, by virtual of which nics are in use on a given node. A test is
added to verify this option.
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-May/359188.html
Signed-off-by: Darrell Ball
---
v3: Make manpage comments more verbose.
Expand commit message comments.
v2: Per particular requirement, support 'no-tcp-seq-chk' rather than
'liberal' mode.
NEWS| 3 ++
lib/conntrack-private.h | 6 ++-
lib/conntrack-tcp.c | 13 -
lib/conntrack.c | 16 ++
lib/conntrack.h | 2 +
lib/ct-dpif.c | 16 ++
lib/ct-dpif.h | 2 +
lib/dpctl.c | 64 +-
lib/dpctl.man | 18 +++
lib/dpif-netdev.c | 18 +++
lib/dpif-netlink.c | 2 +
lib/dpif-provider.h | 5 ++
tests/ofproto-dpif.at | 138
13 files changed, 298 insertions(+), 5 deletions(-)
diff --git a/NEWS b/NEWS
index aba2e4a..3b72d1b 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ Post-v2.12.0
* OVN has been removed from this repository. It now exists as a
separate project. You can find it at
https://github.com/ovn-org/ovn.git
+ - Userspace datapath:
+ * Add option to enable, disable and query TCP sequence checking in
+ conntrack.
v2.12.0 - 03 Sep 2019
-
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index bcfbe10..7cdc2a4 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -171,8 +171,10 @@ struct conntrack {
struct hindex alg_expectation_refs OVS_GUARDED; /* For lookup from
* control context. */
-/* Fragmentation handling context. */
-struct ipf *ipf;
+struct ipf *ipf; /* Fragmentation handling context. */
+atomic_bool tcp_seq_ckk; /* TCP sequence number verification; when
+enabled, this enables sequence number
+verification; enabled by default. */
};
/* Lock acquisition order:
diff --git a/lib/conntrack-tcp.c b/lib/conntrack-tcp.c
index 397aca1..1e843f3 100644
--- a/lib/conntrack-tcp.c
+++ b/lib/conntrack-tcp.c
@@ -39,10 +39,15 @@
#include
#include "conntrack-private.h"
+#include "coverage.h"
#include "ct-dpif.h"
#include "dp-packet.h"
#include "util.h"
+COVERAGE_DEFINE(conntrack_tcp_seq_chk_bypass);
+COVERAGE_DEFINE(conntrack_tcp_seq_chk_failed);
+COVERAGE_DEFINE(conntrack_invalid_tcp_flags);
+
struct tcp_peer {
uint32_t seqlo; /* Max sequence number sent */
uint32_t seqhi; /* Max the other end ACKd + win */
@@ -162,6 +167,7 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_,
uint32_t p_len = tcp_payload_length(pkt);
if (tcp_invalid_flags(tcp_flags)) {
+COVERAGE_INC(conntrack_invalid_tcp_flags);
return CT_UPDATE_INVALID;
}
@@ -272,7 +278,7 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_,
int ackskew = check_ackskew ? dst->seqlo - ack : 0;
#define MAXACKWINDOW (0x + 1500)/* 1500 is an arbitrary fudge factor */
-if (SEQ_GEQ(src->seqhi, end)
+if ((SEQ_GEQ(src->seqhi, end)
/* Last octet inside other's window space */
&& SEQ_GEQ(seq, src->seqlo - (dst->max_win << dws))
/* Retrans: not more than one window back */
@@ -281,7 +287,9 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_,
&& (ackskew <= (MAXACKWINDOW << sws))
/* Acking not more than one window forward */
&& ((tcp_flags & TCP_RST) == 0 || orig_seq == src->seqlo
-|| (orig_seq == src->seqlo + 1) || (orig_seq + 1 == src->seqlo))) {
+|| (orig_seq == src->seqlo + 1) || (orig_seq + 1 == src->seqlo)))
+|| (!conntrack_get_tcp_seq_chk(ct)
+? COVERAGE_INC(conntrack_tcp_seq_chk_bypass), 1 : 0)) {
/* Require an exact/+1 sequence match on resets when possible */
/* update max window */
@@ -385,6 +393,7 @@ tcp_conn_update(struct conntrack *ct, struct conn *conn_,
src->state = dst->state = CT_DPIF_TCPS_TIME_WAIT;
}
} else {
+COVERAGE_INC(conntrack_tcp_seq_chk_failed);
return CT_UPDATE_INVALID;
}
diff --git a/lib/conntrack.c b/lib/conntrack.c
index b56ef06..755934a 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -310,6 +310,7 @@ conntrack_init(void)
Re: [ovs-dev] [patch v2] conntrack: Add option to disable TCP sequence checking.
Thanks Ben I just noticed I sent another version here https://patchwork.ozlabs.org/patch/1153283/ but there were a couple of issues that deserve a resend. It might need a rebase as well Darrell On Tue, Sep 24, 2019 at 3:18 PM Ben Pfaff wrote: > On Wed, Jun 12, 2019 at 12:44:18PM -0700, Darrell Ball wrote: > > On Wed, Jun 12, 2019 at 10:58 AM Ben Pfaff wrote: > > > > > On Wed, Jun 12, 2019 at 10:31:17AM -0700, Darrell Ball wrote: > > > > On Wed, Jun 12, 2019 at 10:09 AM Ben Pfaff wrote: > > > > > > > > > On Wed, Jun 12, 2019 at 08:46:06AM -0700, Darrell Ball wrote: > > > > > > On Mon, Jun 10, 2019 at 9:51 AM Ben Pfaff wrote: > > > > > > > > > > > > > On Sun, Jun 09, 2019 at 07:35:09AM -0700, Darrell Ball wrote: > > > > > > > > This may be needed in some special cases, such as to support > some > > > > > > > > hardware offload implementations. > > > > > > > > > > > > > > > > Reported-at: > > > > > > > > > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-May/359188.html > > > > > > > > Signed-off-by: Darrell Ball > > > > > > > > --- > > > > > > > > > > > > > > > > v2: Per particular requirement, support 'no-tcp-seq-chk' > rather > > > than > > > > > > > > 'liberal' mode. > > > > > > > > > > > > > > > > Add some debug counters. > > > > > > > > > > > > > > I'm not sure whether an ovs-appctl command is the best way for > > > users to > > > > > > > enable and disable this. It means that it is difficult for an > > > OpenFlow > > > > > > > controller to do it, since those commands aren't exposed via > > > OpenFlow > > > > > or > > > > > > > OVSDB. > > > > > > > > > > > > > > > > > > > Thanks for your comments > > > > > > > > > > > > For local controller usage, we are using ovs-appctl today in > similar > > > > > cases > > > > > > for existing products. > > > > > > > > > > > > In the case of non-local controller usage, the remote controller > > > would > > > > > need > > > > > > remote access. > > > > > > > > > > > > However, in this case, I don't expect the remote controller to > be > > > > > > involved; I was assuming > > > > > > that a deployment script would be used to set the value to > > > non-default > > > > > > value (in needed cases) > > > > > > when ovs-vswitchd is (re)started only. If this assumption cannot > be > > > > > > satisfied then we would > > > > > > have to have to introduce a dependency on the database for these > > > types of > > > > > > commands. > > > > > > > > > > This seems to be teetering toward the pre-SDN model of having to > > > > > separately configure each switch. Do you have some rationale in > mind > > > > > why this should be a per-node decision rather than one made by the > > > > > controller? > > > > > > > > > > > > 1/ Because of the reduced security implications vs higher performance > > > > advantage, it would be a per node (or per node role) decision of > whether > > > > to use it or not. > > > > > > Are you saying that the only advantage of disabling TCP sequence > > > checking is performance, and only in the presence of hardware for > > > offloading that requires it? > > > > > > Some HWOL implementations would be the most common 'recommended' usage. > > I will be adding a general statement to the documentation and will echo > it > > in the commit > > message. > > Is there a v3 with that change? I haven't been able to find it. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v2] conntrack: Add option to disable TCP sequence checking.
On Wed, Jun 12, 2019 at 12:44:18PM -0700, Darrell Ball wrote: > On Wed, Jun 12, 2019 at 10:58 AM Ben Pfaff wrote: > > > On Wed, Jun 12, 2019 at 10:31:17AM -0700, Darrell Ball wrote: > > > On Wed, Jun 12, 2019 at 10:09 AM Ben Pfaff wrote: > > > > > > > On Wed, Jun 12, 2019 at 08:46:06AM -0700, Darrell Ball wrote: > > > > > On Mon, Jun 10, 2019 at 9:51 AM Ben Pfaff wrote: > > > > > > > > > > > On Sun, Jun 09, 2019 at 07:35:09AM -0700, Darrell Ball wrote: > > > > > > > This may be needed in some special cases, such as to support some > > > > > > > hardware offload implementations. > > > > > > > > > > > > > > Reported-at: > > > > > > > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-May/359188.html > > > > > > > Signed-off-by: Darrell Ball > > > > > > > --- > > > > > > > > > > > > > > v2: Per particular requirement, support 'no-tcp-seq-chk' rather > > than > > > > > > > 'liberal' mode. > > > > > > > > > > > > > > Add some debug counters. > > > > > > > > > > > > I'm not sure whether an ovs-appctl command is the best way for > > users to > > > > > > enable and disable this. It means that it is difficult for an > > OpenFlow > > > > > > controller to do it, since those commands aren't exposed via > > OpenFlow > > > > or > > > > > > OVSDB. > > > > > > > > > > > > > > > > Thanks for your comments > > > > > > > > > > For local controller usage, we are using ovs-appctl today in similar > > > > cases > > > > > for existing products. > > > > > > > > > > In the case of non-local controller usage, the remote controller > > would > > > > need > > > > > remote access. > > > > > > > > > > However, in this case, I don't expect the remote controller to be > > > > > involved; I was assuming > > > > > that a deployment script would be used to set the value to > > non-default > > > > > value (in needed cases) > > > > > when ovs-vswitchd is (re)started only. If this assumption cannot be > > > > > satisfied then we would > > > > > have to have to introduce a dependency on the database for these > > types of > > > > > commands. > > > > > > > > This seems to be teetering toward the pre-SDN model of having to > > > > separately configure each switch. Do you have some rationale in mind > > > > why this should be a per-node decision rather than one made by the > > > > controller? > > > > > > > > > 1/ Because of the reduced security implications vs higher performance > > > advantage, it would be a per node (or per node role) decision of whether > > > to use it or not. > > > > Are you saying that the only advantage of disabling TCP sequence > > checking is performance, and only in the presence of hardware for > > offloading that requires it? > > > Some HWOL implementations would be the most common 'recommended' usage. > I will be adding a general statement to the documentation and will echo it > in the commit > message. Is there a v3 with that change? I haven't been able to find it. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v2] conntrack: Fix 'reverse_nat_packet()' variable datatype.
On Fri, Aug 30, 2019 at 09:13:19AM -0700, Darrell Ball wrote:
> The datatype 'pad' in the function 'reverse_nat_packet()' was incorrectly
> declared as 'char' instead of 'uint8_t'. This can affect reverse natting
> of icmpX packets with padding > 127 bytes. At the same time, add some
> comments regarding 'extract_l3_ipvX' usage in this function. Found by
> inspection.
>
> Fixes: edd1bef468c0 ("dpdk: Add more ICMP Related NAT support.")
> Signed-off-by: Darrell Ball
Applied to master and backported as far as branch-2.8.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Fix 'check_orig_tuple()' Valgrind false positive.
Done!
On Tue, Sep 24, 2019 at 02:56:40PM -0700, Darrell Ball wrote:
> Thanks Ben
>
> Would you mind applying to 2.12 as well.
>
> Darrell
>
> On Tue, Sep 24, 2019 at 2:34 PM Ben Pfaff wrote:
>
> > On Mon, Sep 23, 2019 at 04:44:33PM -0700, Darrell Ball wrote:
> > > Valgrind reported that 'pkt->md.ct_orig_tuple.ipv4.ipv4_proto' is
> > > uninitialized in 'check_orig_tuple()', if 'ct_state' is zero. Although
> > > this is true, the check is superceded, as even if it succeeds the check
> > > for natted packets based on 'ct_state' is an ORed condition and is
> > intended
> > > to catch this case.
> > > The check is '!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT))' which
> > > filters out all packets excepted natted ones. Move this check up to
> > > prevent the Valgrind complaint, which also helps performance and also
> > remove
> > > recenlty added redundant check adding extra cycles.
> > >
> > > Fixes: f44733c527da ("conntrack: Validate accessing of conntrack data in
> > pkt_metadata.")
> > > CC: Yifeng Sun
> > > Signed-off-by: Darrell Ball
> >
> > Thanks, applied to master.
> >
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Fix 'check_orig_tuple()' Valgrind false positive.
Thanks Ben
Would you mind applying to 2.12 as well.
Darrell
On Tue, Sep 24, 2019 at 2:34 PM Ben Pfaff wrote:
> On Mon, Sep 23, 2019 at 04:44:33PM -0700, Darrell Ball wrote:
> > Valgrind reported that 'pkt->md.ct_orig_tuple.ipv4.ipv4_proto' is
> > uninitialized in 'check_orig_tuple()', if 'ct_state' is zero. Although
> > this is true, the check is superceded, as even if it succeeds the check
> > for natted packets based on 'ct_state' is an ORed condition and is
> intended
> > to catch this case.
> > The check is '!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT))' which
> > filters out all packets excepted natted ones. Move this check up to
> > prevent the Valgrind complaint, which also helps performance and also
> remove
> > recenlty added redundant check adding extra cycles.
> >
> > Fixes: f44733c527da ("conntrack: Validate accessing of conntrack data in
> pkt_metadata.")
> > CC: Yifeng Sun
> > Signed-off-by: Darrell Ball
>
> Thanks, applied to master.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovsdb-idlc.in: fix dict change during iteration.
On Sat, Sep 14, 2019 at 08:17:28PM -0300, Flavio Leitner via dev wrote: > Python3 complains if a dict key is changed during the > iteration. > > Use list() to create a copy of it. Thanks! Applied to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [patch v1] conntrack: Fix 'check_orig_tuple()' Valgrind false positive.
On Mon, Sep 23, 2019 at 04:44:33PM -0700, Darrell Ball wrote:
> Valgrind reported that 'pkt->md.ct_orig_tuple.ipv4.ipv4_proto' is
> uninitialized in 'check_orig_tuple()', if 'ct_state' is zero. Although
> this is true, the check is superceded, as even if it succeeds the check
> for natted packets based on 'ct_state' is an ORed condition and is intended
> to catch this case.
> The check is '!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT))' which
> filters out all packets excepted natted ones. Move this check up to
> prevent the Valgrind complaint, which also helps performance and also remove
> recenlty added redundant check adding extra cycles.
>
> Fixes: f44733c527da ("conntrack: Validate accessing of conntrack data in
> pkt_metadata.")
> CC: Yifeng Sun
> Signed-off-by: Darrell Ball
Thanks, applied to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] odp-util: fill IPv4 ver and head length for tnl_push
On Tue, Sep 24, 2019 at 01:26:42AM +0800, martinbj2...@gmail.com wrote: > From: Martin Zhang > > When parse tnl_push, if IPv4 is used, > we forget to fill the ipv4 version and ip header length fields. > > so there is a wrong ip header in the header of "struct > ovs_action_push_tnl", > which will caused wrong packdet sent by dpcl. > > test command: > ovs-appctl dpctl/add-flow "in_port(1),eth_type(0x0800),ipv4(dst=9.9.9.6)" > \ > > "tnl_push(tnl_port(2),header(size=50,type=4,eth(dst=08:00:27:2e:87:0d,src=98:03:9b:c6:d1:7c,dl_type=0x0800), > \ > > ipv4(src=10.97.240.147,dst=10.96.74.33,proto=17,tos=0,ttl=64,frag=0x4000), \ > > udp(src=0,dst=4789,csum=0x0),vxlan(flags=0x800,vni=0x270f)),out_port(3)),4" > > Signed-off-by: Martin Zhang > Signed-off-by: Dujie Thanks, applied to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3] Learn the mac binding only if required
Bleep bloop. Greetings Numan Siddique, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 98 characters long (recommended limit is 79) #990 FILE: ovn-architecture.7.xml:974: R = lookup_arp(P, A, M); WARNING: Line is 97 characters long (recommended limit is 79) #991 FILE: ovn-architecture.7.xml:975: R = lookup_nd(P, A, M); WARNING: Line is 87 characters long (recommended limit is 79) #998 FILE: ovn-architecture.7.xml:982: 66, then its actions set the logical flow flag MLF_LOOKUP_MAC. WARNING: Line is 91 characters long (recommended limit is 79) #1019 FILE: ovn-sb.xml:1401: R = lookup_arp(P, A, M); WARNING: Line is 92 characters long (recommended limit is 79) #1054 FILE: ovn-sb.xml:1585: R = lookup_nd(P, A, M); Lines checked: 1547, Warnings: 5, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v3] Learn the mac binding only if required
From: Numan Siddique
OVN has the actions - put_arp and put_nd to learn the mac bindings from the
ARP/ND packets. These actions update the Southbound MAC_Binding table.
These actions translates to controller actions. Whenever pinctrl thread
receives such packets, it wakes up the main ovn-controller thread.
If the MAC_Binding table is already upto date, this results
in unnecessary CPU cyles. There are some security implications as well.
A rogue VM can flood broadcast ARP request/reply packets and this
could cause DoS issues. A physical switch may send periodic GARPs
and these packets hit ovn-controllers.
This patch solves these problems by learning the mac bindings only if
required. There is no need to apply the put_arp/put_nd action if the
Southbound MAC_Binding row is upto date.
New actions - lookup_arp and lookup_nd are added which looks up the
IP, MAC pair in the mac_binding table and stores the result in a
register. 1 if lookup is successful, 0 otherwise.
ovn-northd adds 2 new stages - LOOKUP_NEIGHBOR and LEARN_NEIGHBOR before
IP_INPUT in the router ingress pipeline.c. The LOOKUP_NEIGHBOR stage
adds flows to do the lookup in the mac_binding table and the LEARN_NEIGHBOR
adds flows to learn the neighbors only if require.
The lflow module of ovn-controller adds OF flows in table 67
(OFTABLE_MAC_LOOKUP)
for each mac_binding entry with the match reg0 = ip && eth.src = mac with
the action - load:1->reg10[6]
Eg:
table=31,
priority=100,arp,reg0=0xaca8006f,reg14=0x3,metadata=0x3,dl_src=00:44:00:00:00:04
actions=load:1->NXM_NX_REG10[6]
This patch should also address the issue reported in 'Reported-at'
Reported-at: https://bugzilla.redhat.com/1729846
Reported-by: Haidong Li
CC: Han ZHou
CC: Dumitru Ceara
Tested-by: Dumitru Ceara
Signed-off-by: Numan Siddique
---
v2 -> v3
* Addressed review comments from Han.
v1 -> v2
* Addressed review comments from Han - Storing the result
of lookup_arp/lookup_nd in a register.
controller/lflow.c | 37 -
controller/lflow.h | 1 +
include/ovn/actions.h| 13 ++
include/ovn/logical-fields.h | 4 +
lib/actions.c| 114 ++
northd/ovn-northd.8.xml | 212 -
northd/ovn-northd.c | 205 ++---
ovn-architecture.7.xml | 18 +++
ovn-sb.xml | 57 +++
tests/ovn.at | 290 ++-
tests/test-ovn.c | 1 +
utilities/ovn-trace.c| 69 +
12 files changed, 844 insertions(+), 177 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index d0335a83a..e3ed20cd4 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -687,6 +687,7 @@ consider_logical_flow(
.egress_ptable = OFTABLE_LOG_EGRESS_PIPELINE,
.output_ptable = output_ptable,
.mac_bind_ptable = OFTABLE_MAC_BINDING,
+.mac_lookup_ptable = OFTABLE_MAC_LOOKUP,
};
ovnacts_encode(ovnacts.data, ovnacts.size, , );
ovnacts_free(ovnacts.data, ovnacts.size);
@@ -777,7 +778,9 @@ consider_neighbor_flow(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
return;
}
-struct match match = MATCH_CATCHALL_INITIALIZER;
+struct match get_arp_match = MATCH_CATCHALL_INITIALIZER;
+struct match lookup_arp_match = MATCH_CATCHALL_INITIALIZER;
+
if (strchr(b->ip, '.')) {
ovs_be32 ip;
if (!ip_parse(b->ip, )) {
@@ -785,7 +788,9 @@ consider_neighbor_flow(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
VLOG_WARN_RL(, "bad 'ip' %s", b->ip);
return;
}
-match_set_reg(, 0, ntohl(ip));
+match_set_reg(_arp_match, 0, ntohl(ip));
+match_set_reg(_arp_match, 0, ntohl(ip));
+match_set_dl_type(_arp_match, htons(ETH_TYPE_ARP));
} else {
struct in6_addr ip6;
if (!ipv6_parse(b->ip, )) {
@@ -795,17 +800,35 @@ consider_neighbor_flow(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
}
ovs_be128 value;
memcpy(, , sizeof(value));
-match_set_xxreg(, 0, ntoh128(value));
+match_set_xxreg(_arp_match, 0, ntoh128(value));
+
+match_set_xxreg(_arp_match, 0, ntoh128(value));
+match_set_dl_type(_arp_match, htons(ETH_TYPE_IPV6));
+match_set_nw_proto(_arp_match, 58);
+match_set_icmp_code(_arp_match, 0);
}
-match_set_metadata(, htonll(pb->datapath->tunnel_key));
-match_set_reg(, MFF_LOG_OUTPORT - MFF_REG0, pb->tunnel_key);
+match_set_metadata(_arp_match, htonll(pb->datapath->tunnel_key));
+match_set_reg(_arp_match, MFF_LOG_OUTPORT - MFF_REG0, pb->tunnel_key);
+
+match_set_metadata(_arp_match, htonll(pb->datapath->tunnel_key));
+match_set_reg(_arp_match, MFF_LOG_INPORT - MFF_REG0,
+ pb->tunnel_key);
uint64_t stub[1024 / 8];
struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
Re: [ovs-dev] [PATCH ovn v2] Learn the mac binding only if required
Thanks Han for the reviews. Please see below for some comments. Thanks Numan On Wed, Sep 18, 2019 at 4:56 AM Han Zhou wrote: > > On Mon, Sep 16, 2019 at 10:17 AM wrote: > > > > From: Numan Siddique > > > > OVN has the actions - put_arp and put_nd to learn the mac bindings from > the > > ARP/ND packets. These actions update the Southbound MAC_Binding table. > > These actions translates to controller actions. Whenever pinctrl thread > > receives such packets, it wakes up the main ovn-controller thread. > > If the MAC_Binding table is already upto date, this results > > in unnecessary CPU cyles. There are some security implications as well. > > A rogue VM can flood broadcast ARP request/reply packets and this > > could cause DoS issues. A physical switch may send periodic GARPs > > and these packets hit ovn-controllers. > > > > This patch solves these problems by learning the mac bindings only if > > required. There is no need to apply the put_arp/put_nd action if the > > Southbound MAC_Binding row is upto date. > > > > New actions - lookup_arp and lookup_nd are added which looks up the > > IP, MAC pair in the mac_binding table and stores the result in a > > register. 1 if lookup is successful, 0 otherwise. > > > > ovn-northd adds 2 new stages - lookup_arp and put_arp before ip_input > > in the router ingress pipeline. > > > > The logical flows looks something like: > > > > table=1 (lr_in_lookup_arp), priority=100 , match=(arp), > > reg9[4] = lookup_arp(inport, arp.spa, arp.sha); next;) > > > > table=1 (lr_in_lookup_arp), priority=0, match=(1), action=(next;) > > ... > > table=2 (lr_in_put_arp ), priority=100 , > > match=(arp.op == 2 && reg9[4] == 0), > > action=(put_arp(inport, arp.spa, arp.sha);) > > table=2 (lr_in_put_arp ), priority=90 , match=(arp.op == 2), > action=(drop;) > > table=2 (lr_in_put_arp ), priority=0, match=(1), action=(next;) > > > > The lflow module of ovn-controller adds OF flows in table 31 > (OFTABLE_MAC_LOOKUP) > > for each mac_binding entry with the match reg0 = ip && eth.src = mac with > > the action - load:1->reg2[0] > > > > Eg: > > table=31, > priority=100,arp,reg0=0xaca8006f,reg14=0x3,metadata=0x3,dl_src=00:44:00:00:00:04 > > actions=load:1->NXM_NX_REG2[0] > > > > This patch should also address the issue reported in 'Reported-at' > > > > Reported-at: https://bugzilla.redhat.com/1729846 > > Reported-by: Haidong Li > > CC: Han ZHou > > CC: Dumitru Ceara > > Tested-by: Dumitru Ceara > > Signed-off-by: Numan Siddique > > --- > > > > v1 -> v2 > > === > >* Addressed review comments from Han - Storing the result > > of lookup_arp/lookup_nd in a register. > > > > controller/lflow.c | 36 - > > controller/lflow.h | 1 + > > include/ovn/actions.h| 13 ++ > > include/ovn/logical-fields.h | 3 + > > lib/actions.c| 115 ++ > > northd/ovn-northd.8.xml | 251 -- > > northd/ovn-northd.c | 205 ++--- > > ovn-sb.xml | 57 +++ > > tests/ovn.at | 290 ++- > > tests/test-ovn.c | 1 + > > utilities/ovn-trace.c| 69 + > > 11 files changed, 861 insertions(+), 180 deletions(-) > > > > Hi Numan, > > This looks great. I spent more time on the review and here are my comments: > > 1. #define MFF_LOG_LOOKUP_MAC MFF_REG2 > > This new logical field is overlapping with the logical registers, as > defined: > > /* Logical registers. >* >* Make sure these don't overlap with the logical fields! */ > #define MFF_LOG_REG0 MFF_REG0 > #define MFF_N_LOG_REGS 10 > > REG0 - REG9 are already reserved by above definition. If we have to use > one, it should be REG9, and then update MFF_N_LOG_REGS to 9. However, I > think it is better to use just one bit from MFF_LOG_FLAGS. The bits of the > flag is defined as MLF macros, and we can define a new one for > MLF_LOOKUP_MAC_BIT. > In addition, this change needs to be documented in ovn-architecture. > Agree. I will do. > > 2. #define OFTABLE_MAC_LOOKUP 31 > > Table 31 is the last table of logical flows. This mac lookup table is not > part of logical flows, so it is better to use table 67. > I thought about this. Since the action lookup_arp/lookup_nd uses inport where as the action put_arp use outport , i thought it is better to handle this with in table 31. But I think it should be fine if we refer to inport (reg14) in table 67. I will modify to table 67. > 3. lookup_arp/lookup_nd needs to be documented in ovn-architecture, at the > same place where put_arp/get_arp, etc. are documented. > Ack. > > 4. In ovn-northd.xml, the term "MAC learning" is better to be changed to > something like "MAC-binding learning", or just "Neigbour learning", because > "MAC learning" usually means MAC-port table populating in L2 switch in > networking terminology. It
Re: [ovs-dev] [PATCH 1/1] ovs-ofctl: fix memory leak in open_vconn__() function
On Tue, Sep 24, 2019 at 03:41:22PM +0200, Damijan Skvarc wrote: > Signed-off-by: Damijan Skvarc Thanks. I applied this to master. There's another memory leak here (in the run()) call, but it's even less worth fixing than this one. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 ovn 3/3] northd: introduce logical flow for localnet egress shaping
Add set_queue() action for qos capable localnet port in
S_SWITCH_OUT_PORT_SEC_L2 stage of logical swith pipeline
Introduce build_lswitch_{input,outpur}_port_sec and refactor
lswitch_port_security code in order to remove duplicated code
Signed-off-by: Lorenzo Bianconi
---
northd/ovn-northd.8.xml | 7 +-
northd/ovn-northd.c | 231
2 files changed, 143 insertions(+), 95 deletions(-)
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 0f4f1c112..093b55438 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1150,10 +1150,15 @@ output;
eth.dst are always accepted instead of being subject to the
port security rules; this is implemented through a priority-100 flow that
matches on eth.mcast with action output;.
- Finally, to ensure that even broadcast and multicast packets are not
+ Moreover, to ensure that even broadcast and multicast packets are not
delivered to disabled logical ports, a priority-150 flow for each
disabled logical outport overrides the priority-100 flow
with a drop; action.
+ Finally if egress qos has been enabled on a localnet port, the outgoing
+ queue id is set through set_queue action. Please remember to
+ mark the corresponding physical interface with
+ ovn-egress-iface set to true in
Logical Router Datapaths
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 633fb502b..b1ece2d29 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3903,6 +3903,140 @@ has_stateful_acl(struct ovn_datapath *od)
return false;
}
+static void
+build_lswitch_input_port_sec(struct hmap *ports, struct hmap *datapaths,
+ struct hmap *lflows)
+{
+/* Logical switch ingress table 0: Ingress port security - L2
+ * (priority 50).
+ * Ingress table 1: Ingress port security - IP (priority 90 and 80)
+ * Ingress table 2: Ingress port security - ND (priority 90 and 80)
+ */
+struct ds actions = DS_EMPTY_INITIALIZER;
+struct ds match = DS_EMPTY_INITIALIZER;
+struct ovn_port *op;
+
+HMAP_FOR_EACH (op, key_node, ports) {
+if (!op->nbsp) {
+continue;
+}
+
+if (!lsp_is_enabled(op->nbsp)) {
+/* Drop packets from disabled logical ports (since logical flow
+ * tables are default-drop). */
+continue;
+}
+
+if (lsp_is_external(op->nbsp)) {
+continue;
+}
+
+ds_clear();
+ds_clear();
+ds_put_format(, "inport == %s", op->json_key);
+build_port_security_l2("eth.src", op->ps_addrs, op->n_ps_addrs,
+ );
+
+const char *queue_id = smap_get(>sb->options, "qdisc_queue_id");
+if (queue_id) {
+ds_put_format(, "set_queue(%s); ", queue_id);
+}
+ds_put_cstr(, "next;");
+ovn_lflow_add(lflows, op->od, S_SWITCH_IN_PORT_SEC_L2, 50,
+ ds_cstr(), ds_cstr());
+
+if (op->nbsp->n_port_security) {
+build_port_security_ip(P_IN, op, lflows);
+build_port_security_nd(op, lflows);
+}
+}
+
+/* Ingress table 1 and 2: Port security - IP and ND, by default
+ * goto next. (priority 0)
+ */
+struct ovn_datapath *od;
+HMAP_FOR_EACH (od, key_node, datapaths) {
+if (!od->nbs) {
+continue;
+}
+
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_ND, 0, "1", "next;");
+ovn_lflow_add(lflows, od, S_SWITCH_IN_PORT_SEC_IP, 0, "1", "next;");
+}
+
+ds_destroy();
+ds_destroy();
+}
+
+static void
+build_lswitch_output_port_sec(struct hmap *ports, struct hmap *datapaths,
+ struct hmap *lflows)
+{
+struct ds actions = DS_EMPTY_INITIALIZER;
+struct ds match = DS_EMPTY_INITIALIZER;
+struct ovn_port *op;
+
+/* Egress table 8: Egress port security - IP (priorities 90 and 80)
+ * if port security enabled.
+ *
+ * Egress table 9: Egress port security - L2 (priorities 50 and 150).
+ *
+ * Priority 50 rules implement port security for enabled logical port.
+ *
+ * Priority 150 rules drop packets to disabled logical ports, so that
+ * they don't even receive multicast or broadcast packets.
+ */
+HMAP_FOR_EACH (op, key_node, ports) {
+if (!op->nbsp || lsp_is_external(op->nbsp)) {
+continue;
+}
+
+ds_clear();
+ds_clear();
+
+ds_put_format(, "outport == %s", op->json_key);
+if (lsp_is_enabled(op->nbsp)) {
+build_port_security_l2("eth.dst", op->ps_addrs, op->n_ps_addrs,
+ );
+
+if (!strcmp(op->nbsp->type, "localnet")) {
+const char *queue_id = smap_get(>sb->options,
+"qdisc_queue_id");
+if (queue_id) {
+
[ovs-dev] [PATCH v2 ovn 2/3] northd: add the possibility to define localnet as qos capable port
Refactor allocate_chassis_queueid and free_chassis_queueid in order
to get an unused queue_id even for localnet ports and add the
the possibility to define localnet as qos capable port
Acked-by: Dumitru Ceara
Signed-off-by: Lorenzo Bianconi
---
northd/ovn-northd.c | 45 ++---
1 file changed, 30 insertions(+), 15 deletions(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index f393cebb8..633fb502b 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -357,7 +357,7 @@ destroy_chassis_queues(struct hmap *set)
}
static void
-add_chassis_queue(struct hmap *set, struct uuid *chassis_uuid,
+add_chassis_queue(struct hmap *set, const struct uuid *chassis_uuid,
uint32_t queue_id)
{
struct ovn_chassis_qdisc_queues *node = xmalloc(sizeof *node);
@@ -368,7 +368,7 @@ add_chassis_queue(struct hmap *set, struct uuid
*chassis_uuid,
}
static bool
-chassis_queueid_in_use(const struct hmap *set, struct uuid *chassis_uuid,
+chassis_queueid_in_use(const struct hmap *set, const struct uuid *chassis_uuid,
uint32_t queue_id)
{
const struct ovn_chassis_qdisc_queues *node;
@@ -383,31 +383,38 @@ chassis_queueid_in_use(const struct hmap *set, struct
uuid *chassis_uuid,
}
static uint32_t
-allocate_chassis_queueid(struct hmap *set, struct sbrec_chassis *chassis)
+allocate_chassis_queueid(struct hmap *set, const struct uuid *uuid, char *name)
{
+if (!uuid) {
+return 0;
+}
+
for (uint32_t queue_id = QDISC_MIN_QUEUE_ID + 1;
queue_id <= QDISC_MAX_QUEUE_ID;
queue_id++) {
-if (!chassis_queueid_in_use(set, >header_.uuid, queue_id)) {
-add_chassis_queue(set, >header_.uuid, queue_id);
+if (!chassis_queueid_in_use(set, uuid, queue_id)) {
+add_chassis_queue(set, uuid, queue_id);
return queue_id;
}
}
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-VLOG_WARN_RL(, "all %s queue ids exhausted", chassis->name);
+VLOG_WARN_RL(, "all %s queue ids exhausted", name);
return 0;
}
static void
-free_chassis_queueid(struct hmap *set, struct sbrec_chassis *chassis,
+free_chassis_queueid(struct hmap *set, const struct uuid *uuid,
uint32_t queue_id)
{
-const struct uuid *chassis_uuid = >header_.uuid;
+if (!uuid) {
+return;
+}
+
struct ovn_chassis_qdisc_queues *node;
HMAP_FOR_EACH_WITH_HASH (node, key_node,
- hash_chassis_queue(chassis_uuid, queue_id), set) {
-if (uuid_equals(chassis_uuid, >chassis_uuid)
+ hash_chassis_queue(uuid, queue_id), set) {
+if (uuid_equals(uuid, >chassis_uuid)
&& node->queue_id == queue_id) {
hmap_remove(set, >key_node);
free(node);
@@ -2650,15 +2657,23 @@ ovn_port_update_sbrec(struct northd_context *ctx,
uint32_t queue_id = smap_get_int(
>sb->options, "qdisc_queue_id", 0);
bool has_qos = port_has_qos_params(>nbsp->options);
+const struct uuid *uuid = NULL;
struct smap options;
+char *name = "";
+
+if (!strcmp(op->nbsp->type, "localnet")) {
+uuid = >sb->header_.uuid;
+name = "localnet";
+} else if (op->sb->chassis) {
+uuid = >sb->chassis->header_.uuid;
+name = op->sb->chassis->name;
+}
-if (op->sb->chassis && has_qos && !queue_id) {
+if (has_qos && !queue_id) {
queue_id = allocate_chassis_queueid(chassis_qdisc_queues,
-op->sb->chassis);
+uuid, name);
} else if (!has_qos && queue_id) {
-free_chassis_queueid(chassis_qdisc_queues,
- op->sb->chassis,
- queue_id);
+free_chassis_queueid(chassis_qdisc_queues, uuid, queue_id);
queue_id = 0;
}
--
2.21.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 ovn 1/3] Add egress QoS mapping for non-tunnel interfaces
Introduce add_localnet_egress_interface_mappings routine in order to collect as
egress interfaces all ovs bridge interfaces marked with ovn-egress-iface
in the external_ids column of ovs interface table.
ovn-egress-iface is used to indicate to which localnet ports QoS egress
shaping has to be applied.
Refactor add_bridge_mappings routine
Signed-off-by: Lorenzo Bianconi
---
controller/binding.c| 51 -
controller/binding.h| 4 ++
controller/ovn-controller.c | 3 +-
controller/patch.c | 76 +
controller/patch.h | 4 ++
5 files changed, 103 insertions(+), 35 deletions(-)
diff --git a/controller/binding.c b/controller/binding.c
index 242163d59..aad9d39e6 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -18,6 +18,7 @@
#include "ha-chassis.h"
#include "lflow.h"
#include "lport.h"
+#include "patch.h"
#include "lib/bitmap.h"
#include "openvswitch/poll-loop.h"
@@ -532,6 +533,9 @@ consider_local_datapath(struct ovsdb_idl_txn *ovnsb_idl_txn,
/* Add all localnet ports to local_lports so that we allocate ct zones
* for them. */
sset_add(local_lports, binding_rec->logical_port);
+if (qos_map && ovs_idl_txn) {
+get_qos_params(binding_rec, qos_map);
+}
} else if (!strcmp(binding_rec->type, "external")) {
if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
chassis_rec)) {
@@ -619,10 +623,48 @@ consider_local_datapath(struct ovsdb_idl_txn
*ovnsb_idl_txn,
}
}
+static void
+add_localnet_egress_interface_mappings(
+const struct sbrec_port_binding *port_binding,
+struct shash *bridge_mappings, struct sset *egress_ifaces)
+{
+const char *network = smap_get(_binding->options, "network_name");
+if (!network) {
+return;
+}
+
+struct ovsrec_bridge *br_ln = shash_find_data(bridge_mappings, network);
+if (!br_ln) {
+return;
+}
+
+/* Add egress-ifaces from the connected bridge */
+for (size_t i = 0; i < br_ln->n_ports; i++) {
+const struct ovsrec_port *port_rec = br_ln->ports[i];
+
+for (size_t j = 0; j < port_rec->n_interfaces; j++) {
+const struct ovsrec_interface *iface_rec;
+
+iface_rec = port_rec->interfaces[j];
+bool is_egress_iface = smap_get_bool(_rec->external_ids,
+ "ovn-egress-iface", false);
+if (!is_egress_iface) {
+continue;
+}
+sset_add(egress_ifaces, iface_rec->name);
+}
+}
+}
+
static void
consider_localnet_port(const struct sbrec_port_binding *binding_rec,
+ struct shash *bridge_mappings,
+ struct sset *egress_ifaces,
struct hmap *local_datapaths)
{
+add_localnet_egress_interface_mappings(binding_rec,
+bridge_mappings, egress_ifaces);
+
struct local_datapath *ld
= get_local_datapath(local_datapaths,
binding_rec->datapath->tunnel_key);
@@ -655,6 +697,8 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
const struct ovsrec_bridge *br_int,
const struct sbrec_chassis *chassis_rec,
const struct sset *active_tunnels,
+const struct ovsrec_bridge_table *bridge_table,
+const struct ovsrec_open_vswitch_table *ovs_table,
struct hmap *local_datapaths, struct sset *local_lports,
struct sset *local_lport_ids)
{
@@ -663,6 +707,7 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
}
const struct sbrec_port_binding *binding_rec;
+struct shash bridge_mappings = SHASH_INITIALIZER(_mappings);
struct shash lport_to_iface = SHASH_INITIALIZER(_to_iface);
struct sset egress_ifaces = SSET_INITIALIZER(_ifaces);
struct hmap qos_map;
@@ -688,14 +733,18 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
}
+add_ovs_bridge_mappings(ovs_table, bridge_table, _mappings);
+
/* Run through each binding record to see if it is a localnet port
* on local datapaths discovered from above loop, and update the
* corresponding local datapath accordingly. */
SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec, port_binding_table) {
if (!strcmp(binding_rec->type, "localnet")) {
-consider_localnet_port(binding_rec, local_datapaths);
+consider_localnet_port(binding_rec, _mappings,
+ _ifaces, local_datapaths);
}
}
+shash_destroy(_mappings);
if (!sset_is_empty(_ifaces)
&& set_noop_qos(ovs_idl_txn, port_table, qos_table, _ifaces)) {
diff --git a/controller/binding.h b/controller/binding.h
index bae162ede..924891c1b 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -26,6 +26,8 @@ struct ovsdb_idl_txn;
struct
[ovs-dev] [PATCH v2 ovn 0/3] Introduce localnet egress QoS support
OVN applies logical switch QoS settings to egress interfaces. It
currently works by analyzing each br-int interface to see what the remote-ip
is on it, and then adding qdiscs to the tunnel-egress-iface associated
with this br-int interface.
This doesn't work as well when working with VLAN networks. In VLAN
networks, the interface on br-int associated with the localnet port on
the logical switch will be a patch port. It's connected to another
bridge, making it difficult to determine which interface (or interfaces) is
the egress interface on that connected bridge.
This series aims to mend this. On the bridge that is patched to br-int,
interfaces can have the boolean external-ids:ovn-egress-iface set to
true if this is an egress interface. This way, QoS can be applied to
these interfaces the same as is applied to tunnel interfaces.
Moreover rework qos code in order to add the capability to select automatically
the queue_id used to identify the device queue
Finally add the set_queue action to logical flows in Egress Port Security - L2
stage
for localnet QoS capable ports in order to set the physical_interface qdisc id
Changes since v1:
- split build_lswitch_port_sec in build_lswitch_input_port_sec and
build_lswitch_output_port_sec
- removed unnecessary log messages
Changes since RFC:
- introduce build_lswitch_port_sec as a container for {in/out}_port_sec logical
flow configuration
- move ovn-egress-iface lookup in consider_localnet_port in order to avoid an
unnecessary port_binding lookup
Lorenzo Bianconi (3):
Add egress QoS mapping for non-tunnel interfaces
northd: add the possibility to define localnet as qos capable port
northd: interoduce logical flow for localnet egress shaping
controller/binding.c| 51 ++-
controller/binding.h| 4 +
controller/ovn-controller.c | 3 +-
controller/patch.c | 76 +-
controller/patch.h | 4 +
northd/ovn-northd.8.xml | 7 +-
northd/ovn-northd.c | 276 ++--
7 files changed, 276 insertions(+), 145 deletions(-)
--
2.21.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCHv4] netdev-afxdp: Add need_wakeup supprt.
Hi. Thanks for a new version. Comments inline. Best regards, Ilya Maximets. On 17.09.2019 21:36, William Tu wrote: The patch adds support for using need_wakeup flag in AF_XDP rings. A new option, use_need_wakeup, is added. When this option is used, it means that OVS has to explicitly wake up the kernel RX, using poll() syscall and wake up TX, using sendto() syscall. This feature improves the performance by avoiding unnecessary sendto syscalls for TX. For RX, instead of kernel always busy-spinning on fille queue, OVS wakes up the kernel RX processing when fill queue is replenished. The need_wakeup feature is merged into Linux kernel 5.3.0-rc1 and OVS enables it by default. Running the feature before this version causes xsk bind fails, please use options:use_need_wakeup=false to disable it. If users enable it but runs in an older version of libbpf, then the need_wakeup feature has no effect, and a warning message is logged. For virtual interface, it's better set use_need_wakeup=false, since the virtual device's AF_XDP xmit is synchronous: the sendto syscall enters kernel and process the TX packet on tx queue directly. I tested on kernel 5.3.0-rc3 using its libbpf. On Intel Xeon E5-2620 v3 2.4GHz system, performance of physical port to physical port improves from 6.1Mpps to 7.3Mpps. Testing on 5.2.0-rc6 using libbpf from 5.3.0-rc3 Kernel versions here all messed up. See the comment below. does not work due to libbpf API change. Users have to use the older libbpf for older kernel. Suggested-by: Ilya Maximets Signed-off-by: William Tu --- v4: - move use_need_wakeup check inside xsk_rx_wakeup_if_needed v3: - add warning when user enables it but libbpf not support it - revise documentation v2: - address feedbacks from Ilya and Eelco - add options:use_need_wakeup, default to true - remove poll timeout=1sec, make poll() return immediately - naming change: rename to xsk_rx_wakeup_if_needing - fix indents and return value for errno --- Documentation/intro/install/afxdp.rst | 15 - acinclude.m4 | 8 +++ lib/netdev-afxdp.c| 104 ++ lib/netdev-linux-private.h| 2 + vswitchd/vswitch.xml | 13 + 5 files changed, 127 insertions(+), 15 deletions(-) diff --git a/Documentation/intro/install/afxdp.rst b/Documentation/intro/install/afxdp.rst index 820e9d993d8f..545516b2bbec 100644 --- a/Documentation/intro/install/afxdp.rst +++ b/Documentation/intro/install/afxdp.rst @@ -176,9 +176,18 @@ in :doc:`general`:: ovs-vswitchd ... ovs-vsctl -- add-br br0 -- set Bridge br0 datapath_type=netdev -Make sure your device driver support AF_XDP, and to use 1 PMD (on core 4) -on 1 queue (queue 0) device, configure these options: **pmd-cpu-mask, -pmd-rxq-affinity, and n_rxq**. The **xdpmode** can be "drv" or "skb":: +Make sure your device driver support AF_XDP, netdev-afxdp supports +the following additional options (see man ovs-vswitchd.conf.db for +more details): + + * **xdpmode**: use "drv" for driver mode, or "skb" for skb mode. + + * **use_need_wakeup**: disable by setting to "false", otherwise default + is "true" + +For example, to use 1 PMD (on core 4) on 1 queue (queue 0) device, +configure these options: **pmd-cpu-mask, pmd-rxq-affinity, and n_rxq**. +The **xdpmode** can be "drv" or "skb":: ethtool -L enp2s0 combined 1 ovs-vsctl set Open_vSwitch . other_config:pmd-cpu-mask=0x10 diff --git a/acinclude.m4 b/acinclude.m4 index f0e38898b17a..df1082c455fc 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -276,6 +276,14 @@ AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [ [Define to 1 if AF_XDP support is available and enabled.]) LIBBPF_LDADD=" -lbpf -lelf" AC_SUBST([LIBBPF_LDADD]) + +AC_CHECK_DECL([xsk_ring_prod__needs_wakeup], [ + AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [1], +[XDP need wakeup support detected in xsk.h.]) +], [ + AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [0], +[XDP need wakeup support not detected in xsk.h.]) + ], [#include ]) fi AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true) ]) diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c index e5b058d08a09..a101a750bc5f 100644 --- a/lib/netdev-afxdp.c +++ b/lib/netdev-afxdp.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -82,7 +83,7 @@ BUILD_ASSERT_DECL(PROD_NUM_DESCS == CONS_NUM_DESCS); #define UMEM2DESC(elem, base) ((uint64_t)((char *)elem - (char *)base)) static struct xsk_socket_info *xsk_configure(int ifindex, int xdp_queue_id, - int mode); + int mode, bool use_need_wakeup); static void xsk_remove_xdp_program(uint32_t ifindex, int xdpmode); static void xsk_destroy(struct xsk_socket_info *xsk); static int xsk_configure_all(struct netdev *netdev); @@ -117,6 +118,54 @@ struct xsk_socket_info
[ovs-dev] Hi
Hi ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v7 1/1] Avoid dp_hash recirculation for balance-tcp bond selection mode
On 17.09.2019 11:50, Vishal Deep Ajmera wrote:> Problem: In OVS-DPDK, flows with output over a bond interface of type “balance-tcp” (using a hash on TCP/UDP 5-tuple) get translated by the ofproto layer into "HASH" and "RECIRC" datapath actions. After recirculation, the packet is forwarded to the bond member port based on 8-bits of the datapath hash value computed through dp_hash. This causes performance degradation in the following ways: 1. The recirculation of the packet implies another lookup of the packet’s flow key in the exact match cache (EMC) and potentially Megaflow classifier (DPCLS). This is the biggest cost factor. 2. The recirculated packets have a new “RSS” hash and compete with the original packets for the scarce number of EMC slots. This implies more EMC misses and potentially EMC thrashing causing costly DPCLS lookups. 3. The 256 extra megaflow entries per bond for dp_hash bond selection put additional load on the revalidation threads. Owing to this performance degradation, deployments stick to “balance-slb” bond mode even though it does not do active-active load balancing for VXLAN- and GRE-tunnelled traffic because all tunnel packet have the same source MAC address. Proposed optimization: -- This proposal introduces a new load-balancing output action instead of recirculation. Maintain one table per-bond (could just be an array of uint16's) and program it the same way internal flows are created today for each possible hash value(256 entries) from ofproto layer. Use this table to load-balance flows as part of output action processing. Currently xlate_normal() -> output_normal() -> bond_update_post_recirc_rules() -> bond_may_recirc() and compose_output_action__() generate “dp_hash(hash_l4(0))” and “recirc()” actions. In this case the RecircID identifies the bond. For the recirculated packets the ofproto layer installs megaflow entries that match on RecircID and masked dp_hash and send them to the corresponding output port. Instead, we will now generate actions as "hash(l4(0)),lb_output(bond,)" This combines hash computation (only if needed, else re-use RSS hash) and inline load-balancing over the bond. This action is used *only* for balance-tcp bonds in OVS-DPDK datapath (the OVS kernel datapath remains unchanged). Example: Current scheme: --- With 1 IP-UDP flow: flow-dump from pmd on cpu core: 2 recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth(src=02:00:00:02:14:01,dst=0c:c4:7a:58:f0:2b),eth_type(0x0800),ipv4(frag=no), packets:2828969, bytes:181054016, used:0.000s, actions:hash(hash_l4(0)),recirc(0x1) recirc_id(0x1),dp_hash(0x113683bd/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:2828937, bytes:181051968, used:0.000s, actions:2 With 8 IP-UDP flows (with random UDP src port): (all hitting same DPCL): flow-dump from pmd on cpu core: 2 recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth(src=02:00:00:02:14:01,dst=0c:c4:7a:58:f0:2b),eth_type(0x0800),ipv4(frag=no), packets:2674009, bytes:171136576, used:0.000s, actions:hash(hash_l4(0)),recirc(0x1) recirc_id(0x1),dp_hash(0xf8e02b7e/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:377395, bytes:24153280, used:0.000s, actions:2 recirc_id(0x1),dp_hash(0xb236c260/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:333486, bytes:21343104, used:0.000s, actions:1 recirc_id(0x1),dp_hash(0x7d89eb18/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:348461, bytes:22301504, used:0.000s, actions:1 recirc_id(0x1),dp_hash(0xa78d75df/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:633353, bytes:40534592, used:0.000s, actions:2 recirc_id(0x1),dp_hash(0xb58d846f/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:319901, bytes:20473664, used:0.001s, actions:2 recirc_id(0x1),dp_hash(0x24534406/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:334985, bytes:21439040, used:0.001s, actions:1 recirc_id(0x1),dp_hash(0x3cf32550/0xff),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:326404, bytes:20889856, used:0.001s, actions:1 New scheme: --- We can do with a single flow entry (for any number of new flows): in_port(7),packet_type(ns=0,id=0),eth(src=02:00:00:02:14:01,dst=0c:c4:7a:58:f0:2b),eth_type(0x0800),ipv4(frag=no), packets:2674009, bytes:171136576, used:0.000s, actions:hash(l4(0)),lb_output(bond,1) A new CLI has been added to dump datapath bond cache as given below. “sudo ovs-appctl dpif-netdev/dp-bond-show [dp]” root@ubuntu-190:performance_scripts # sudo ovs-appctl dpif-netdev/dp-bond-show Bond cache: bond-id 1 : bucket 0 - slave 2 bucket 1 - slave 1 bucket 2 - slave 2 bucket 3 - slave 1 Performance improvement: With a prototype of
[ovs-dev] [PATCH 1/1] ovs-ofctl: fix memory leak in open_vconn__() function
Signed-off-by: Damijan Skvarc
---
utilities/ovs-ofctl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 06289d2..b2350e0 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -590,6 +590,8 @@ open_vconn__(const char *name, enum open_target target,
} else if (!open_vconn_socket(socket_name, vconnp)) {
/* Fall Through. */
} else {
+free(bridge_path);
+free(socket_name);
ovs_fatal(0, "%s is not a bridge or a socket", name);
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] faq: Update OVS/DPDK version table for OVS 2.12.
On 23.09.2019 23:55, Ben Pfaff wrote: On Mon, Sep 23, 2019 at 04:59:11PM +0100, Kevin Traynor wrote: Indicate that OVS 2.12 uses DPDK 18.11.2. Signed-off-by: Kevin Traynor --- Can also be backported to branch-2.12. Thanks! Done. Thanks, Kevin and Ben for taking care of this! Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] netdev-dpdk: add support for the RTE_ETH_EVENT_INTR_RESET event
On 12 Sep 2019, at 12:24, Ilya Maximets wrote:
On 12.09.2019 13:19, Ilya Maximets wrote:
On 12.09.2019 13:07, Eelco Chaudron wrote:
On 12 Sep 2019, at 10:39, Ilya Maximets wrote:
On 11.09.2019 16:20, Eelco Chaudron wrote:
Currently, OVS does not register and therefore not handle the
interface reset event from the DPDK framework. This would cause a
problem in cases where a VF is used as an interface, and its
configuration changes.
As an example in the following scenario the MAC change is not
detected/acted upon until OVS is restarted without the patch
applied:
$ echo 1 > /sys/bus/pci/devices/:05:00.1/sriov_numvfs
$ ovs-vsctl add-port ovs_pvp_br0 dpdk0 -- \
set Interface dpdk0 type=dpdk -- \
set Interface dpdk0
options:dpdk-devargs=:05:0a.0
$ ip link set p5p2 vf 0 mac 52:54:00:92:d3:33
Signed-off-by: Eelco Chaudron
---
@@ -4180,13 +4223,19 @@ netdev_dpdk_reconfigure(struct netdev
*netdev)
&& dev->rxq_size == dev->requested_rxq_size
&& dev->txq_size == dev->requested_txq_size
&& dev->socket_id == dev->requested_socket_id
- && dev->started) {
+ && dev->started && !dev->reset_needed) {
/* Reconfiguration is unnecessary */
goto out;
}
- rte_eth_dev_stop(dev->port_id);
+ if (dev->reset_needed) {
+ rte_eth_dev_reset(dev->port_id);
Thinking more about the change and looking at flow control
configuration,
it seems that on reset we'll lost configurations done in
set_config().
Device reset should return it to initial state, i.e. all the
default settings,
but set_config() will not be called after that.
I know, that VFs commonly doesn't support flow control, but if
we'll add like
real configuration of MAC address, it will be lost too.
What do you think?
Doesn’t the full bridge run sequence take care of this?
So in callback we do netdev_request_reconfigure() which triggers the
following sequence…
bridge_run__()
...
dpif_netdev_run()
...
reconfigure_datapath()
...
netdev_dpdk_reconfigure()
But than also it will call in the next run
bridge_run()
...
bridge_delete_or_reconfigure_ports()
...
netdev_set_config()
netdev_dpdk_set_config()
Or do I miss something?
dpif_netdev_run() is called from ofproto_type_run() which is called
unconditionally from the bridge_run().
But bridge_delete_or_reconfigure_ports() only called from
bridge_reconfigure(),
which is protected by the following condition:
if (ovsdb_idl_get_seqno(idl) != idl_seqno ||
if_notifier_changed(ifnotifier)) {
i.e. if there will be no DB updates or there will be no netlink
notifications,
set_config() will not be called. I understand that in your scenario
there
might be netlink notification for interface changes since PF is
controlled
by the kernel, but it might be not the case if PF attached as a DPDK
port
to OVS or some other application.
Hmm. Basically, dpdk_eth_event_callback() is an analogue of the
if_notifier, but for DPDK application.
Maybe we can add it as another trigger for above 'if' condition?
It sounds like a good idea, however, I see no clean way of doing this as
the if_notifier_changed()/if_notifier_create() is not data path
abstracted.
Any ideas for a nice clean solution here? Guess we could add a
dpdk_notifier_create() in if-notifier.c
Thoughts?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [RFC} netdev-dpdk: setting VF MAC address
On cloud topology with hardware offload, when SR-IOV is used there is an architecture limitation to configure the VF from the host. The port representor is attached to OVS, while the VF is usually configured in pass through to the VM. In such topology the MAC address of the VF is configured by the infra (OpenStack for example), as VM is usually untrusted. For OVS-kernel this can be done by ip link command line. For OVS-DPDK and in fact for any DPDK application that doesn't use bifurcated driver, the only entity that can configure the VF MAC address is the PF owner. Since the DPDK ports management is controlled by the application (OVS here), such configuration request should be intercepted by OVS. A more complex approach would be to have an external tool, provided by DPDK, which would take care of avoiding any conflict or race condition with the application threads. It is obviously easier to follow the current DPDK model and keep full control in OVS. A new command line for OVS will be added: ovs-vsctl set interface mac=XX:XX:XX:XX:XX:XX The OVS will use the rte_eth_dev_default_mac_addr_set api that should be extended to configure the VF mac address in the name of the representor. see link: https://www.mail-archive.com/dev@dpdk.org/msg140671.html It seems like this local approach requires minor changes in OVS netdev-dpdk, and the benefit will be substantial mainly OVS-DPDK integration with OpenStack and other controllers. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 2/3] northd: add the possibility to define localnet as qos capable port
On Mon, Sep 23, 2019 at 5:09 PM Lorenzo Bianconi
wrote:
>
> Refactor allocate_chassis_queueid and free_chassis_queueid in order
> to get an unused queue_id even for localnet ports and add the
> the possibility to define localnet as qos capable port
>
> Signed-off-by: Lorenzo Bianconi
Looks good to me.
Acked-by: Dumitru Ceara
> ---
> northd/ovn-northd.c | 45 ++---
> 1 file changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index f393cebb8..633fb502b 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -357,7 +357,7 @@ destroy_chassis_queues(struct hmap *set)
> }
>
> static void
> -add_chassis_queue(struct hmap *set, struct uuid *chassis_uuid,
> +add_chassis_queue(struct hmap *set, const struct uuid *chassis_uuid,
>uint32_t queue_id)
> {
> struct ovn_chassis_qdisc_queues *node = xmalloc(sizeof *node);
> @@ -368,7 +368,7 @@ add_chassis_queue(struct hmap *set, struct uuid
> *chassis_uuid,
> }
>
> static bool
> -chassis_queueid_in_use(const struct hmap *set, struct uuid *chassis_uuid,
> +chassis_queueid_in_use(const struct hmap *set, const struct uuid
> *chassis_uuid,
> uint32_t queue_id)
> {
> const struct ovn_chassis_qdisc_queues *node;
> @@ -383,31 +383,38 @@ chassis_queueid_in_use(const struct hmap *set, struct
> uuid *chassis_uuid,
> }
>
> static uint32_t
> -allocate_chassis_queueid(struct hmap *set, struct sbrec_chassis *chassis)
> +allocate_chassis_queueid(struct hmap *set, const struct uuid *uuid, char
> *name)
> {
> +if (!uuid) {
> +return 0;
> +}
> +
> for (uint32_t queue_id = QDISC_MIN_QUEUE_ID + 1;
> queue_id <= QDISC_MAX_QUEUE_ID;
> queue_id++) {
> -if (!chassis_queueid_in_use(set, >header_.uuid, queue_id)) {
> -add_chassis_queue(set, >header_.uuid, queue_id);
> +if (!chassis_queueid_in_use(set, uuid, queue_id)) {
> +add_chassis_queue(set, uuid, queue_id);
> return queue_id;
> }
> }
>
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> -VLOG_WARN_RL(, "all %s queue ids exhausted", chassis->name);
> +VLOG_WARN_RL(, "all %s queue ids exhausted", name);
> return 0;
> }
>
> static void
> -free_chassis_queueid(struct hmap *set, struct sbrec_chassis *chassis,
> +free_chassis_queueid(struct hmap *set, const struct uuid *uuid,
> uint32_t queue_id)
> {
> -const struct uuid *chassis_uuid = >header_.uuid;
> +if (!uuid) {
> +return;
> +}
> +
> struct ovn_chassis_qdisc_queues *node;
> HMAP_FOR_EACH_WITH_HASH (node, key_node,
> - hash_chassis_queue(chassis_uuid, queue_id),
> set) {
> -if (uuid_equals(chassis_uuid, >chassis_uuid)
> + hash_chassis_queue(uuid, queue_id), set) {
> +if (uuid_equals(uuid, >chassis_uuid)
> && node->queue_id == queue_id) {
> hmap_remove(set, >key_node);
> free(node);
> @@ -2650,15 +2657,23 @@ ovn_port_update_sbrec(struct northd_context *ctx,
> uint32_t queue_id = smap_get_int(
> >sb->options, "qdisc_queue_id", 0);
> bool has_qos = port_has_qos_params(>nbsp->options);
> +const struct uuid *uuid = NULL;
> struct smap options;
> +char *name = "";
> +
> +if (!strcmp(op->nbsp->type, "localnet")) {
> +uuid = >sb->header_.uuid;
> +name = "localnet";
> +} else if (op->sb->chassis) {
> +uuid = >sb->chassis->header_.uuid;
> +name = op->sb->chassis->name;
> +}
>
> -if (op->sb->chassis && has_qos && !queue_id) {
> +if (has_qos && !queue_id) {
> queue_id = allocate_chassis_queueid(chassis_qdisc_queues,
> -op->sb->chassis);
> +uuid, name);
> } else if (!has_qos && queue_id) {
> -free_chassis_queueid(chassis_qdisc_queues,
> - op->sb->chassis,
> - queue_id);
> +free_chassis_queueid(chassis_qdisc_queues, uuid, queue_id);
> queue_id = 0;
> }
>
> --
> 2.21.0
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 3/3] northd: interoduce logical flow for localnet egress shaping
On Mon, Sep 23, 2019 at 5:09 PM Lorenzo Bianconi
wrote:
>
> Add set_queue() action for qos capable localnet port in
> S_SWITCH_OUT_PORT_SEC_L2 stage of logical swith pipeline
> Introduce build_lswitch_port_sec and refactor lswitch_port_security code
> in order to remove duplicated code
>
> Signed-off-by: Lorenzo Bianconi
There's a typo in the subject line: s/interoduce/Introduce.
> ---
> northd/ovn-northd.8.xml | 7 +-
> northd/ovn-northd.c | 199 +---
> 2 files changed, 110 insertions(+), 96 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 0f4f1c112..093b55438 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1150,10 +1150,15 @@ output;
>eth.dst are always accepted instead of being subject to
> the
>port security rules; this is implemented through a priority-100 flow
> that
>matches on eth.mcast with action output;.
> - Finally, to ensure that even broadcast and multicast packets are not
> + Moreover, to ensure that even broadcast and multicast packets are not
>delivered to disabled logical ports, a priority-150 flow for each
>disabled logical outport overrides the priority-100 flow
>with a drop; action.
> + Finally if egress qos has been enabled on a localnet port, the outgoing
> + queue id is set through set_queue action. Please remember
> to
> + mark the corresponding physical interface with
> + ovn-egress-iface set to true in + table="Interface" db="Open_vSwitch"/>
>
>
> Logical Router Datapaths
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 633fb502b..4bacad572 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -3903,6 +3903,108 @@ has_stateful_acl(struct ovn_datapath *od)
> return false;
> }
>
> +static void
> +build_lswitch_port_sec(struct hmap *ports, struct hmap *datapaths,
> + struct hmap *lflows)
> +{
Isn't it better to split this function into two functions (one for
ingress and one for egress)? What do you think?
> +struct ds match = DS_EMPTY_INITIALIZER;
> +struct ds actions = DS_EMPTY_INITIALIZER;
> +struct ovn_datapath *od;
> +struct ovn_port *op;
> +
> +HMAP_FOR_EACH (op, key_node, ports) {
> +if (!op->nbsp) {
> +continue;
> +}
> +
> +if (lsp_is_external(op->nbsp)) {
> +continue;
> +}
> +
> +ds_clear();
> +ds_put_format(, "outport == %s", op->json_key);
> +
> +/* Egress table 8: Egress port security - IP (priorities 90 and 80)
> + * if port security enabled.
> + *
> + * Egress table 9: Egress port security - L2 (priorities 50 and 150).
> + *
> + * Priority 50 rules implement port security for enabled logical
> port.
> + *
> + * Priority 150 rules drop packets to disabled logical ports, so that
> + * they don't even receive multicast or broadcast packets.
> + */
> +if (!lsp_is_enabled(op->nbsp)) {
> +ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC_L2, 150,
> + ds_cstr(), "drop;");
> +build_port_security_ip(P_OUT, op, lflows);
Do we still need to add port security flows for IP if the port is
disabled? Won't the drop flow above suffice?
Thanks,
Dumitru
> +continue;
> +}
> +
> +ds_clear();
> +if (!strcmp(op->nbsp->type, "localnet")) {
> +const char *queue_id = smap_get(>sb->options,
> +"qdisc_queue_id");
> +if (queue_id) {
> +ds_put_format(, "set_queue(%s); ", queue_id);
> +}
> +}
> +ds_put_cstr(, "output;");
> +build_port_security_l2("eth.dst", op->ps_addrs, op->n_ps_addrs,
> + );
> +ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC_L2, 50,
> + ds_cstr(), ds_cstr());
> +
> +ds_clear();
> +ds_clear();
> +
> +/* Logical switch ingress table 0: Ingress port security - L2
> + * (priority 50).
> + * Ingress table 1: Ingress port security - IP (priority 90 and 80)
> + * Ingress table 2: Ingress port security - ND (priority 90 and 80)
> + */
> +ds_put_format(, "inport == %s", op->json_key);
> +build_port_security_l2("eth.src", op->ps_addrs, op->n_ps_addrs,
> + );
> +
> +const char *queue_id = smap_get(>sb->options, "qdisc_queue_id");
> +if (queue_id) {
> +ds_put_format(, "set_queue(%s); ", queue_id);
> +}
> +ds_put_cstr(, "next;");
> +ovn_lflow_add(lflows, op->od, S_SWITCH_IN_PORT_SEC_L2, 50,
> + ds_cstr(), ds_cstr());
> +
> +if (op->nbsp->n_port_security) {
> +
Re: [ovs-dev] [PATCH ovn 1/3] Add egress QoS mapping for non-tunnel interfaces
On Mon, Sep 23, 2019 at 5:09 PM Lorenzo Bianconi
wrote:
>
> Introduce add_localnet_egress_interface_mappings routine in order to collect
> as
> egress interfaces all ovs bridge interfaces marked with ovn-egress-iface
> in the external_ids column of ovs interface table.
> ovn-egress-iface is used to indicate to which localnet ports QoS egress
> shaping has to be applied.
> Refactor add_bridge_mappings routine
>
> Signed-off-by: Lorenzo Bianconi
Hi Lorenzo,
This change looks good to me. Just a few minor comments.
Thanks,
Dumitru
> ---
> controller/binding.c| 58 +++-
> controller/binding.h| 4 ++
> controller/ovn-controller.c | 3 +-
> controller/patch.c | 76 +
> controller/patch.h | 4 ++
> 5 files changed, 110 insertions(+), 35 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 242163d59..89262b2d2 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -18,6 +18,7 @@
> #include "ha-chassis.h"
> #include "lflow.h"
> #include "lport.h"
> +#include "patch.h"
>
> #include "lib/bitmap.h"
> #include "openvswitch/poll-loop.h"
> @@ -532,6 +533,9 @@ consider_local_datapath(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> /* Add all localnet ports to local_lports so that we allocate ct
> zones
> * for them. */
> sset_add(local_lports, binding_rec->logical_port);
> +if (qos_map && ovs_idl_txn) {
> +get_qos_params(binding_rec, qos_map);
> +}
> } else if (!strcmp(binding_rec->type, "external")) {
> if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
>chassis_rec)) {
> @@ -619,10 +623,55 @@ consider_local_datapath(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> }
> }
>
> +static void
> +add_localnet_egress_interface_mappings(
> +const struct sbrec_port_binding *port_binding,
> +struct shash *bridge_mappings, struct sset *egress_ifaces)
> +{
> +const char *network = smap_get(_binding->options, "network_name");
> +if (!network) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +VLOG_ERR_RL(, "%s port '%s' has no network name.",
> + port_binding->type, port_binding->logical_port);
We already log this in patch_run() -> add_bridge_mappings(). I think
we can skip the logging and just return.
> +return;
> +}
> +
> +struct ovsrec_bridge *br_ln = shash_find_data(bridge_mappings, network);
> +if (!br_ln) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> +VLOG_ERR_RL(, "bridge not found for %s port '%s' "
> +"with network name '%s'", port_binding->type,
> +port_binding->logical_port, network);
Same here.
> +return;
> +}
> +
> +/* Add egress-ifaces from the connected bridge */
> +for (size_t i = 0; i < br_ln->n_ports; i++) {
> +const struct ovsrec_port *port_rec = br_ln->ports[i];
> +
> +for (size_t j = 0; j < port_rec->n_interfaces; j++) {
> +const struct ovsrec_interface *iface_rec;
> +
> +iface_rec = port_rec->interfaces[j];
> +bool is_egress_iface = smap_get_bool(_rec->external_ids,
> + "ovn-egress-iface", false);
> +if (!is_egress_iface) {
> +continue;
> +}
> +sset_add(egress_ifaces, iface_rec->name);
> +}
> +}
> +}
> +
> static void
> consider_localnet_port(const struct sbrec_port_binding *binding_rec,
> + struct shash *bridge_mappings,
> + struct sset *egress_ifaces,
> struct hmap *local_datapaths)
> {
> +add_localnet_egress_interface_mappings(binding_rec,
> +bridge_mappings, egress_ifaces);
> +
> struct local_datapath *ld
> = get_local_datapath(local_datapaths,
> binding_rec->datapath->tunnel_key);
> @@ -655,6 +704,8 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> const struct ovsrec_bridge *br_int,
> const struct sbrec_chassis *chassis_rec,
> const struct sset *active_tunnels,
> +const struct ovsrec_bridge_table *bridge_table,
> +const struct ovsrec_open_vswitch_table *ovs_table,
> struct hmap *local_datapaths, struct sset *local_lports,
> struct sset *local_lport_ids)
> {
> @@ -663,6 +714,7 @@ binding_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> }
>
> const struct sbrec_port_binding *binding_rec;
> +struct shash bridge_mappings = SHASH_INITIALIZER(_mappings);
> struct shash lport_to_iface = SHASH_INITIALIZER(_to_iface);
> struct sset egress_ifaces = SSET_INITIALIZER(_ifaces);
> struct hmap qos_map;
> @@ -688,14 +740,18 @@ binding_run(struct
Re: [ovs-dev] [PATCH ovn] Fix the compilation failures
On Tue, Sep 24, 2019 at 2:09 AM Russell Bryant wrote:
> Sorry.
>
>
No worries :)
Thanks for the review. I applied this patch to master.
Numan
> Acked-by: Russell Bryant
>
> > On Sep 23, 2019, at 4:20 PM, nusid...@redhat.com wrote:
> >
> > From: Numan Siddique
> >
> > Below compilation errors are seen:
> >
> > - make[1]: *** No rule to make target
> 'Documentation/internals/charter.rst', needed by 'all-am'. Stop.
> >
> > - Warning, treated as error:
> > ../MAINTAINERS.rst:63:Insufficient data supplied (1 row(s)); no data
> remaining for table body, required by "list-table" directive.
> >
> > Fixes: 0ba67050dcb3("Remove the OVS charter.")
> > Fixes: 311b1a31ceb5(Acknowledge that OVN committers are a new group.)
> > CC: Russell Bryant
> > Signed-off-by: Numan Siddique
> > ---
> > Documentation/automake.mk | 1 -
> > Documentation/index.rst | 3 +--
> > Documentation/internals/index.rst | 1 -
> > MAINTAINERS.rst | 2 +-
> > 4 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/automake.mk b/Documentation/automake.mk
> > index f7e1d2628..ff376fd83 100644
> > --- a/Documentation/automake.mk
> > +++ b/Documentation/automake.mk
> > @@ -39,7 +39,6 @@ DOC_SOURCE = \
> >Documentation/internals/index.rst \
> >Documentation/internals/authors.rst \
> >Documentation/internals/bugs.rst \
> > -Documentation/internals/charter.rst \
> >Documentation/internals/committer-emeritus-status.rst \
> >Documentation/internals/committer-grant-revocation.rst \
> >Documentation/internals/committer-responsibilities.rst \
> > diff --git a/Documentation/index.rst b/Documentation/index.rst
> > index de4c45857..290c0abdd 100644
> > --- a/Documentation/index.rst
> > +++ b/Documentation/index.rst
> > @@ -85,8 +85,7 @@ Learn more about the Open vSwitch project and about
> how you can contribute:
> > :doc:`internals/contributing/coding-style` |
> > :doc:`internals/contributing/coding-style-windows`
> >
> > -- **Maintaining:** :doc:`internals/charter` |
> > - :doc:`internals/maintainers` |
> > +- **Maintaining:** :doc:`internals/maintainers` |
> > :doc:`internals/committer-responsibilities` |
> > :doc:`internals/committer-grant-revocation` |
> > :doc:`internals/committer-emeritus-status`
> > diff --git a/Documentation/internals/index.rst
> b/Documentation/internals/index.rst
> > index 1da7501e2..cf54d74b3 100644
> > --- a/Documentation/internals/index.rst
> > +++ b/Documentation/internals/index.rst
> > @@ -39,7 +39,6 @@ itself and how they might involved.
> >release-process
> >bugs
> >security
> > - charter
> >committer-emeritus-status
> >committer-responsibilities
> >committer-grant-revocation
> > diff --git a/MAINTAINERS.rst b/MAINTAINERS.rst
> > index f2c3e3ecd..59ad1ea57 100644
> > --- a/MAINTAINERS.rst
> > +++ b/MAINTAINERS.rst
> > @@ -61,7 +61,7 @@ More information about Emeritus Committers can be found
> > `here `__.
> >
> > .. list-table:: OVS Emeritus Maintainers
> > - :header-rows: 1
> > + :header-rows: 0
> >
> >* - Name
> > - Email
> > --
> > 2.21.0
> >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2] ovn-northd: Add static IP multicast flood configuration
Add the following new configuration options to the
Logical_Switch_Port:options column in the OVN Northbound database:
- mcast_flood: if set to 'true' all incoming IP multicast traffic
(except IP multicast reports) entering the switch will also be
flooded on the logical switch port.
- mcast_flood_reports: if set to 'true' all incoming IP multicast
entering the switch will also be flooded on the logical switch
port. A clone of the packets is also processed by ovn-controller
for snooping.
Add the following new configuration option to the
Logical_Router_Port:options column in the OVN Northbound database:
- mcast_flood: if set to 'true' all incoming IP multicast traffic
(including IP multicast reports) entering the router will be also
flooded on the logical router port.
Due to the fact that in the router pipeline multicast reports are
not treated in a special way there's no need for an explicit
'mcast_flood_reports' option for router ports.
Signed-off-by: Dumitru Ceara
---
v2: Rebase & fix tag in ovn-nb.xml
---
lib/mcast-group-index.h | 2 +
northd/ovn-northd.8.xml | 30 +--
northd/ovn-northd.c | 212 ++--
ovn-nb.xml | 33
tests/ovn.at| 81 +-
5 files changed, 324 insertions(+), 34 deletions(-)
diff --git a/lib/mcast-group-index.h b/lib/mcast-group-index.h
index cb49ad7..ba995ba 100644
--- a/lib/mcast-group-index.h
+++ b/lib/mcast-group-index.h
@@ -28,6 +28,8 @@ enum ovn_mcast_tunnel_keys {
OVN_MCAST_FLOOD_TUNNEL_KEY = OVN_MIN_MULTICAST,
OVN_MCAST_UNKNOWN_TUNNEL_KEY,
OVN_MCAST_MROUTER_FLOOD_TUNNEL_KEY,
+OVN_MCAST_MROUTER_STATIC_TUNNEL_KEY,
+OVN_MCAST_STATIC_TUNNEL_KEY,
OVN_MIN_IP_MULTICAST,
OVN_MAX_IP_MULTICAST = OVN_MAX_MULTICAST,
};
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 0f4f1c1..429ed7e 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -954,7 +954,11 @@ output;
A priority-100 flow that punts all IGMP packets to
ovn-controller if IGMP snooping is enabled on the
-logical switch.
+logical switch. The flow also forwards the IGMP packets to the
+MC_MROUTER_STATIC multicast group, which
+ovn-northd populates with all the logical ports that
+have
+:mcast_flood_reports='true'.
@@ -976,10 +980,15 @@ output;
A priority-80 flow that forwards all unregistered IP multicast traffic
-to the MC_MROUTER_FLOOD multicast group, if any.
-Otherwise the flow drops all unregistered IP multicast packets. This
-flow is added only if :mcast_flood_unregistered='false'.
+to the MC_STATIC multicast group, which
+ovn-northd populates with all the logical ports that
+have
+:mcast_flood='true'. The flow also forwards
+unregistered IP multicast traffic to the MC_MROUTER_FLOOD
+multicast group, which ovn-northd populates with all the
+logical ports connected to logical routers that have
+
+:mcast_relay='true'.
@@ -2027,6 +2036,17 @@ output;
+ Priority-450 flow that matches unregistered IP multicast traffic
+ and sets outport to the MC_STATIC
+ multicast group, which ovn-northd populates with the
+ logical ports that have
+
+ :mcast_flood='true'.
+
+
+
+
+
For distributed logical routers where one of the logical router
ports specifies a redirect-chassis, a priority-400
logical flow for each ip source/destination couple that matches the
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index f393ceb..538daa1 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -448,7 +448,12 @@ struct mcast_switch_info {
* should be flooded to the mrouter. Only
* applicable if flood_unregistered == false.
*/
-
+bool flood_reports; /* True if the switch has at least one port
+ * configured to flood reports.
+ */
+bool flood_static; /* True if the switch has at least one port
+ * configured to flood traffic.
+ */
int64_t table_size; /* Max number of IP multicast groups. */
int64_t idle_timeout; /* Timeout after which an idle group is
* flushed.
@@ -466,7 +471,10 @@ struct mcast_switch_info {
};
struct mcast_router_info {
-bool relay; /* True if the router should relay IP multicast. */
+bool relay;/* True if the router should relay IP multicast. */
+bool flood_static; /* True if the router has at least one port configured
+* to
Re: [ovs-dev] [PATCH v3] vswitchd: Make packet-in controller queue size configurable
On Tue, Sep 24, 2019 at 1:39 AM Ben Pfaff wrote:
>
> Sorry about that. Applied, thanks!
Thanks!
>
> On Mon, Sep 16, 2019 at 12:24:24PM +0200, Dumitru Ceara wrote:
> > Hi,
> >
> > Just a reminder, Mark has acked this change a while ago but it didn't
> > get pushed yet.
> >
> > Thanks,
> > Dumitru
> >
> > On Fri, Aug 9, 2019 at 5:08 PM Mark Michelson wrote:
> > >
> > > Acked-by: Mark Michelson
> > >
> > > On 8/2/19 4:29 AM, Dumitru Ceara wrote:
> > > > The ofconn packet-in queue for packets that can't be immediately sent
> > > > on the rconn connection was limited to 100 packets (hardcoded value).
> > > > While increasing this limit is usually not recommended as it might
> > > > create buffer bloat and increase latency, in scaled scenarios it is
> > > > useful if the administrator (or CMS) can adjust the queue size.
> > > >
> > > > One such situation was noticed while performing scale testing of the
> > > > OVN IGMP functionality: triggering ~200 simultaneous IGMP reports
> > > > was causing tail drops on the packet-in queue towards ovn-controller.
> > > >
> > > > This commit adds the possibility to configure the queue size for:
> > > > - management controller (br-int.mgmt): through the
> > > >other_config:controller-queue-size column of the Bridge table. This
> > > >value is limited to 512 as large queues definitely affect latency. If
> > > >not present the default value of 100 is used. This is done in order
> > > > to
> > > >maintain the same default behavior as before the commit.
> > > > - other controllers: through the controller_queue_size column of the
> > > >Controller table. This value is also limited to 512. If not present
> > > >the code uses the Bridge:other_config:controller-queue-size
> > > >configuration.
> > > >
> > > > Signed-off-by: Dumitru Ceara
> > > >
> > > > ---
> > > > v3: Remove trailing whitespace.
> > > > v2: Address reviewer comments (increase vswitch.vsschema version &
> > > > reword docs).
> > > > ---
> > > > ofproto/connmgr.c | 6 +-
> > > > ofproto/ofproto.h | 1 +
> > > > vswitchd/bridge.c | 26 ++
> > > > vswitchd/vswitch.ovsschema | 9 +++--
> > > > vswitchd/vswitch.xml | 21 +
> > > > 5 files changed, 60 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> > > > index d975a53..51d656c 100644
> > > > --- a/ofproto/connmgr.c
> > > > +++ b/ofproto/connmgr.c
> > > > @@ -74,6 +74,7 @@ struct ofconn {
> > > > enum ofputil_packet_in_format packet_in_format;
> > > >
> > > > /* OFPT_PACKET_IN related data. */
> > > > +int packet_in_queue_size;
> > > > struct rconn_packet_counter *packet_in_counter; /* # queued on
> > > > 'rconn'. */
> > > > #define N_SCHEDULERS 2
> > > > struct pinsched *schedulers[N_SCHEDULERS];
> > > > @@ -1176,6 +1177,7 @@ ofconn_create(struct ofservice *ofservice, struct
> > > > rconn *rconn,
> > > > ofconn_set_protocol(ofconn, OFPUTIL_P_NONE);
> > > > ofconn->packet_in_format = OFPUTIL_PACKET_IN_STD;
> > > >
> > > > +ofconn->packet_in_queue_size = settings->max_pktq_size;
> > > > ofconn->packet_in_counter = rconn_packet_counter_create();
> > > > ofconn->miss_send_len = (ofconn->type == OFCONN_PRIMARY
> > > >? OFP_DEFAULT_MISS_SEND_LEN
> > > > @@ -1263,6 +1265,7 @@ ofconn_reconfigure(struct ofconn *ofconn, const
> > > > struct ofproto_controller *c)
> > > > rconn_set_probe_interval(ofconn->rconn, probe_interval);
> > > >
> > > > ofconn->band = c->band;
> > > > +ofconn->packet_in_queue_size = c->max_pktq_size;
> > > >
> > > > ofconn_set_rate_limit(ofconn, c->rate_limit, c->burst_limit);
> > > >
> > > > @@ -1676,7 +1679,8 @@ do_send_packet_ins(struct ofconn *ofconn, struct
> > > > ovs_list *txq)
> > > >
> > > > LIST_FOR_EACH_POP (pin, list_node, txq) {
> > > > if (rconn_send_with_limit(ofconn->rconn, pin,
> > > > - ofconn->packet_in_counter, 100) ==
> > > > EAGAIN) {
> > > > + ofconn->packet_in_counter,
> > > > + ofconn->packet_in_queue_size) ==
> > > > EAGAIN) {
> > > > static struct vlog_rate_limit rll =
> > > > VLOG_RATE_LIMIT_INIT(5, 5);
> > > >
> > > > VLOG_INFO_RL(, "%s: dropping packet-in due to queue
> > > > overflow",
> > > > diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> > > > index 6e4afff..b53ea03 100644
> > > > --- a/ofproto/ofproto.h
> > > > +++ b/ofproto/ofproto.h
> > > > @@ -234,6 +234,7 @@ struct ofproto_controller {
> > > >* be negotiated for a session. */
> > > >
> > > > /* OpenFlow packet-in rate-limiting. */
> > > > +int max_pktq_size; /* Maximum number of packet-in to be
> > > > queued. */
> > > > int rate_limit; /* Max
[ovs-dev] [PATCH] odp-util: fill IPv4 ver and head length for tnl_push
From: Martin Zhang
When parse tnl_push, if IPv4 is used,
we forget to fill the ipv4 version and ip header length fields.
so there is a wrong ip header in the header of "struct ovs_action_push_tnl",
which will caused wrong packdet sent by dpcl.
test command:
ovs-appctl dpctl/add-flow "in_port(1),eth_type(0x0800),ipv4(dst=9.9.9.6)" \
"tnl_push(tnl_port(2),header(size=50,type=4,eth(dst=08:00:27:2e:87:0d,src=98:03:9b:c6:d1:7c,dl_type=0x0800),
\
ipv4(src=10.97.240.147,dst=10.96.74.33,proto=17,tos=0,ttl=64,frag=0x4000), \
udp(src=0,dst=4789,csum=0x0),vxlan(flags=0x800,vni=0x270f)),out_port(3)),4"
Signed-off-by: Martin Zhang
Signed-off-by: Dujie
---
lib/odp-util.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 84ea4c1..fe59a56 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1527,6 +1527,7 @@ ovs_parse_tnl_push(const char *s, struct
ovs_action_push_tnl *data)
put_16aligned_be32(>ip_src, sip);
put_16aligned_be32(>ip_dst, dip);
ip->ip_frag_off = htons(ip_frag_off);
+ip->ip_ihl_ver = IP_IHL_VER(5, 4);
ip_len = sizeof *ip;
} else {
char sip6_s[IPV6_SCAN_LEN + 1];
--
1.8.3.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions
Hi Ben & Ilya, The provided changes were internally tested and works fine. Can we consider these changes for now and can be later enhanced as suggested by Ilya. Best Regards, Anil Kumar. -Original Message- From: Anil Kumar Koli Sent: Friday, 13 September, 2019 08:20 AM To: 'Ben Pfaff' Cc: 'Ilya Maximets' ; 'ovs-dev@openvswitch.org' Subject: RE: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions Hi Ben, Please consider this as a gentle remainder and provide your inputs on the below issue. Thanks & Regards, Anil Kumar -Original Message- From: Anil Kumar Koli Sent: Thursday, 5 September, 2019 11:12 PM To: 'Ben Pfaff' Cc: 'Ilya Maximets' ; 'ovs-dev@openvswitch.org' Subject: RE: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions Hi Ben, Please consider this as a gentle remainder and provide your inputs on the below issue. This issue has been discussed earlier in the following thread. https://mail.openvswitch.org/pipermail/ovs-discuss/2019-April/048494.html Best Regards, Anil Kumar. -Original Message- From: Anil Kumar Koli Sent: Tuesday, 3 September, 2019 11:35 AM To: 'Ilya Maximets' ; 'ovs-dev@openvswitch.org' Cc: 'Ben Pfaff' Subject: RE: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions Hi Ilya, Sorry for late reply and thanks for the review comments. Yes, the issue happens only in the userspace datapath and not in kernel datapath. I have tested it on kernel datapath and could not see this issue for the same build. Regarding the suggested solution, I still feel like going with recursive mutex approach is more cleaner then releasing the lock for ofproto_packet_out_finish(). We have the ofproto_dpif_xcache_execute() which may lead to race when a flow mod learn is called from another thread. May be it could also be possible that in certain combination we may result the crash in ofproto_packet_out_start(). May be Ben can share his inputs as well. Best Regards,Anil Kumar -Original Message- From: Ilya Maximets Sent: Thursday, 29 August, 2019 05:10 PM To: Anil Kumar Koli ; ovs-dev@openvswitch.org Cc: 'Ben Pfaff' Subject: Re: [ovs-dev] [PATCH v1] ofproto: Fix OVS crash when packets hit Openflow rules with certain combinations of nested actions On 29.08.2019 12:52, Anil Kumar Koli wrote: > Hi Ilya, > > Thanks for the review. > Please find below the stack trace of the crash > > (gdb) bt > #0 0x7f0a3da46c37 in __GI_raise (sig=sig@entry=6) at > ../nptl/sysdeps/unix/sysv/linux/raise.c:56 > #1 0x7f0a3da4a028 in __GI_abort () at abort.c:89 > #2 0x0094262e in ovs_abort_valist (err_no=, > format=, args=args@entry=0x7fffaf59e308) at > lib/util.c:419 > #3 0x009426b7 in ovs_abort (err_no=, > format=format@entry=0xb0289f "%s: pthread_%s_%s failed") at > lib/util.c:411 > #4 0x009104f9 in ovs_mutex_lock_at (l_=l_@entry=0xe47cc0 > , where=where@entry=0xad4199 "ofproto/ofproto.c:5391") > at lib/ovs-thread.c:75 > #5 0x0083fb1f in ofproto_flow_mod_learn > (ofm=ofm@entry=0x7fffaf59e620, keep_ref=, > limit=, below_limitp=below_limitp@entry=0x7fffaf59e540) > at ofproto/ofproto.c:5391 > #6 0x0085e5d0 in xlate_learn_action > (ctx=ctx@entry=0x7fffaf5a02e0, learn=learn@entry=0x2b18308) at > ofproto/ofproto-dpif-xlate.c:5378 > #7 0x008615c3 in do_xlate_actions (ofpacts=, > ofpacts_len=, ctx=, > is_last_action=, group_bucket_action=) > at ofproto/ofproto-dpif-xlate.c:6893 > #8 0x0085edba in xlate_recursively (actions_xlator=0x860730 > , is_last_action=false, deepens=, > rule=0x2b8e440, ctx=0x7fffaf5a02e0) at > ofproto/ofproto-dpif-xlate.c:4233 > #9 xlate_table_action (ctx=0x7fffaf5a02e0, in_port=, > table_id=, may_packet_in=, > honor_table_miss=, with_ct_orig=, > is_last_action=false, > xlator=0x860730 ) at > ofproto/ofproto-dpif-xlate.c:4361 > #10 0x00861aaa in xlate_ofpact_resubmit (resubmit= out>, resubmit=, resubmit=, > is_last_action=, ctx=0x7fffaf5a02e0) at > ofproto/ofproto-dpif-xlate.c:4672 > #11 do_xlate_actions (ofpacts=ofpacts@entry=0x2b654e8, > ofpacts_len=ofpacts_len@entry=48, ctx=ctx@entry=0x7fffaf5a02e0, > is_last_action=is_last_action@entry=true, > group_bucket_action=group_bucket_action@entry=false) > at ofproto/ofproto-dpif-xlate.c:6773 > #12 0x008696d6 in xlate_actions (xin=xin@entry=0x7fffaf5a0da0, > xout=xout@entry=0x7fffaf5a11b0) at ofproto/ofproto-dpif-xlate.c:7570 > #13 0x00859b56 in upcall_xlate (wc=0x7fffaf5a23f0, > odp_actions=0x7fffaf5a1bc0, upcall=0x7fffaf5a1150, udpif=0x2b10670) at > ofproto/ofproto-dpif-upcall.c:1197 > #14 process_upcall (udpif=udpif@entry=0x2b10670, > upcall=upcall@entry=0x7fffaf5a1150, > odp_actions=odp_actions@entry=0x7fffaf5a1bc0, > wc=wc@entry=0x7fffaf5a23f0) at ofproto/ofproto-dpif-upcall.c:1413 > #15 0x0085a9eb
