Ok, I will make a new version which have no mirror switch. This may need some days.
Thanks Gao Zhenyu <[email protected]> 2017/08/07 22:34 收件人： Ben Pfaff <[email protected]>, 抄送： [email protected], ovs dev <[email protected]>, [email protected], Miguel Angel Ajo Pelayo <[email protected]>, xurong00037997 <[email protected]> 主题： Re: [ovs-dev] 答复: Re: 答复: Re: 答复: Re: [PATCH] ovn: Support for taas(tap-as-a-service) function I agree with Ben. I didn't see a strong requirement that we must need to create a new type of switch for mirroring. We may introduce more and more features in future, some of them may get same request(handle packet in a special way) as well. But introduceing a new type of logcial switch makes the whole logical more complex. So I prefer to use a bit of reg to mark it and follow same pipline. Thanks Zhenyu Gao 2017-08-07 21:41 GMT+08:00 Ben Pfaff <[email protected]>: You keep mentioning "a new type of switch". I don't understand this. Who has proposed adding a new type of switch, and what kind of switch would this be? On Mon, Aug 07, 2017 at 01:37:52PM +0800, [email protected] wrote: > If we do not add a new type of switch, we should write flag to a reg to > indicate the matched packets which are cloned to monitor. > > This reg should add to all the pipeline stages of logical switch(both > ingress and egress) to distinguish from normal flow. Is this right for > Russell's point? > > If we add a new type of switch, we could define a new pipeline like bellow > for the monitor function, this have no influence on normal pipeline. > > /* Logical mirror switch ingress stages. */ \ > PIPELINE_STAGE(MSWITCH, IN, MIRROR_IN, 0, "lms_in_port") \ > PIPELINE_STAGE(MSWITCH, IN, FLOW_FILTER, 1, "lms_in_flow_filter")\ > PIPELINE_STAGE(MSWITCH, IN, OUT_LK, 2, "lms_in_out_lk") \ > \ > /* Logical mirror switch egress stages. */ \ > PIPELINE_STAGE(MSWITCH, OUT, FLOW_FILTER, 0, "lms_out_flow_filter")\ > PIPELINE_STAGE(MSWITCH, OUT, DELIVERY, 1, "lms_out_delivery") > > I think the new defined switch is easy to understand. > > Thanks > > > > > > > Ben Pfaff <[email protected]> > 2017/08/07 11:03 > > 收件人： [email protected], > 抄送： Russell Bryant <[email protected]>, ovs dev > <[email protected]>, [email protected], xurong00037997 > <[email protected]>, Miguel Angel Ajo Pelayo <[email protected]> > 主题： Re: [ovs-dev] 答复: Re: 答复: Re: [PATCH] ovn: Support > for taas(tap-as-a-service) function > > > I am having a very hard time understanding what you're writing here. > Russell's point makes sense to me, but I don't understand your response. > Can you give some examples? > > On Mon, Aug 07, 2017 at 09:40:06AM +0800, [email protected] wrote: > > Not add new logical_mirror_switch, just use logical_switch of course can > > > capture the use case. But logical_switch pipeline is complex for flow > > monitor. Flow monitor should ignore some tables such as port_security, > lb > > and so on. And also should consider normal function for normal ports. I > > think add a new type of switch and the corresponding pipeline may be > more > > clear in logical. > > > > Is there some adverse effect to add new type switch? > > > > Thanks. > > > > > > > > > > Russell Bryant <[email protected]> > > 2017/08/04 22:06 > > > > 收件人： [email protected], > > 抄送： Miguel Angel Ajo Pelayo <[email protected]>, ovs dev > > <[email protected]>, xurong00037997 <[email protected]>, > > [email protected] > > 主题： Re: 答复: Re: [ovs-dev] [PATCH] ovn: Support for > > taas(tap-as-a-service) function > > > > > > > > > > On Thu, Aug 3, 2017 at 8:52 PM, <[email protected]> wrote: > > Miguel Ángel and Russell > > > > Thanks for your reviews. > > > > Current taas function just for port monitor, in this situation, we can > > simplify the design by just add new port type. But we have the plane to > > add flow_classifier to tap_flow to monitor special flows of given port. > > The flow_classifier definition may like as follow: > > 'flow_classifiers': { > > 'id': {'allow_post': False, 'allow_put': False, > > 'validate': {'type:uuid': None}, 'is_visible': True, > > 'primary_key': True}, > > 'tenant_id': {'allow_post': True, 'allow_put': False, > > 'validate': {'type:string': None}, > > 'required_by_policy': True, 'is_visible': True}, > > 'name': {'allow_post': True, 'allow_put': True, > > 'validate': {'type:string': None}, > > 'is_visible': True, 'default': ''}, > > 'description': {'allow_post': True, 'allow_put': True, > > 'validate': {'type:string': None}, > > 'is_visible': True, 'default': ''}, > > 'protocol': {'allow_post': True, 'allow_put': True, > > 'validate': {'type:string': None}, > > 'is_visible': True, 'default': ''}, > > 'src_port_range_min': {'allow_post': True, 'allow_put': True, > > 'convert_to': attr.convert_to_int, > > 'is_visible': True, 'default': 0}, > > 'src_port_range_max': {'allow_post': True, 'allow_put': True, > > 'convert_to': attr.convert_to_int, > > 'is_visible': True, 'default': 0}, > > 'dst_port_range_min': {'allow_post': True, 'allow_put': True, > > 'convert_to': attr.convert_to_int, > > 'is_visible': True, 'default': 0}, > > 'dst_port_range_max': {'allow_post': True, 'allow_put': True, > > 'convert_to': attr.convert_to_int, > > 'is_visible': True, 'default': 0}, > > 'src_ip_prefix': {'allow_post': True, 'allow_put': True, > > 'validate': {'type:subnet': > > attr._validate_subnet}, > > 'is_visible': True, 'default': '0.0.0.0/0'}, > > 'dst_ip_prefix': {'allow_post': True, 'allow_put': True, > > 'validate': {'type:subnet': > > attr._validate_subnet}, > > 'is_visible': True, 'default': '0.0.0.0/0'} > > } > > > > This may need more complex pipeline. So I think add a new table and new > > pipeline may be a easier way. > > > > Thanks for sharing the info on future capabilities. > > > > We have a very flexible syntax for traffic classification in OVN. It's > > the logical flow match syntax (see logical flows in the southbound > > database). We expose this syntax in the northbound database in the > > "match" column of the ACL table. > > > > This would be another use case where we could use this syntax in the > > northbound database. Expanding on my preview proposal: > > > > - a new port type of 'mirror' > > > > - when port type=mirror, an option to identify which port is being > > mirrored > > > > - (the new part) when port type=mirror, an option that may be used to > > specify traffic classification for the subset of traffic on a port to > > mirror, in "match" syntax > > > > Do you think this captures the use case? > > > > -- > > Russell Bryant > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
